Skip to content

Make public methods of the Fraction class - positional-only #101817

Closed as not planned

Description

@skirpichev

For some applications (see gmpy2 users) it still does make sense to use the stdlib rational numbers only as a fallback. However, to make such replacement fully transparent to end users - custom rational type must have a compatible API.

Unfortunately, all public methods of Fraction do support both positional and keyword parameters, that complicates this task (e.g. CPython C-API doesn't provide methods of kwargs parsing for METH_FASTCALL|METH_KEYWORDS, see #83528). I doubt it's necessary and that it was actually used somewhere (BTW, making all arguments positional-only doesn't break the fractions module tests). Quick search (on github) gave me only few examples (<10) of using .limit_denominator(max_denominator=xyz) syntax outside of CPython&forks and nothing about Fraction.from_float(f=xyz) and so on (after all, such calls slightly slower than using positional arguments).

So, lets decide if argument names like f and dec should be part of the Fraction's API forever.

Activity

  1. mdickinson commented on Feb 11, 2023

    @mdickinson
    Member

    So, lets decide if argument names like f and dec should be part of the Fraction's API forever.

    I think it's fine - there's nothing actually wrong here. The maxim If it ain't broke, don't fix it applies to this case. It doesn't seem worth going through all the maintenance effort of adding deprecation warnings and eventually remembering to remove them (or of asking the steering council for an exception).

    Yes, people who are trying to create a 3rd party like-for-like drop-in replacement for Fraction will have to choose either to support the pass-by-name syntax for from_float and from_decimal, or to add a compatibility note that it's not supported. That's up to them. From your research, it seems as though it would be fine for those versions not to support pass-by-name in these cases.

  2. mdickinson commented on Feb 11, 2023

    @mdickinson
    Member

    I'm going to close: the benefit is theoretical rather than practical, and not sufficient to warrant changing the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions