Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

as_integer_ratio() missing from fractions.Fraction() #82000

Closed
rhettinger opened this issue Aug 11, 2019 · 11 comments
Closed

as_integer_ratio() missing from fractions.Fraction() #82000

rhettinger opened this issue Aug 11, 2019 · 11 comments
Labels
3.8 only security fixes 3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 37819
Nosy @gvanrossum, @rhettinger, @mdickinson, @scoder, @ambv, @serhiy-storchaka, @jdemeyer, @lisroach, @CuriousLearner
PRs
  • bpo-37819: Add Fraction.as_integer_ratio() #15212
  • [3.8] bpo-37819: Add Fraction.as_integer_ratio() (GH-15212) #15215
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2019-08-11.22:03:01.198>
    created_at = <Date 2019-08-11.07:01:53.124>
    labels = ['easy', '3.8', 'type-feature', 'library', '3.9']
    title = 'as_integer_ratio() missing from fractions.Fraction()'
    updated_at = <Date 2019-08-19.09:17:27.633>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2019-08-19.09:17:27.633>
    actor = 'jdemeyer'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-08-11.22:03:01.198>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-08-11.07:01:53.124>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37819
    keywords = ['patch', 'easy']
    message_count = 11.0
    messages = ['349378', '349381', '349397', '349398', '349403', '349404', '349411', '349412', '349413', '349936', '349946']
    nosy_count = 9.0
    nosy_names = ['gvanrossum', 'rhettinger', 'mark.dickinson', 'scoder', 'lukasz.langa', 'serhiy.storchaka', 'jdemeyer', 'lisroach', 'CuriousLearner']
    pr_nums = ['15212', '15215']
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue37819'
    versions = ['Python 3.8', 'Python 3.9']

    @rhettinger
    Copy link
    Contributor Author

    When working on Whatsnew3.8, I noticed that as_integer_ratio() had been added to ints and bools, was already present in floats and Decimals, but was missing from Fractions.

    IIRC, the goal was to make all of these have as a similar API so that x.as_integer_ratio() would work for all types that could support it (not complex numbers).

    @rhettinger rhettinger added 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir easy type-feature A feature request or enhancement labels Aug 11, 2019
    @scoder
    Copy link
    Contributor

    scoder commented Aug 11, 2019

    FWIW, makes total sense to me to have it there. Question is more if we can still get it into Py3.8, since it's a new feature for fractions.

    @serhiy-storchaka
    Copy link
    Member

    This will not solve the problem completely, because other rational numbers do not have as_integer_ratio().

    But all rational numbers have the numerator and denominator properties. See bpo-37822 for more general solution.

    @rhettinger
    Copy link
    Contributor Author

    I think this can still go in for Fractions because it completes a feature that was already started (providing the method for the concrete types where it makes sense).

    Altering numbers.py can be saved for the future.

    @rhettinger
    Copy link
    Contributor Author

    Guido, would you like to go forward with this?

    IIRC, you had decided that adding as_integer_ratio() to all the relevant concrete classes was the way to go. We already had the method on floats and Decimals, and then it was added bool and float, so Fraction is the only one missing. I believe the goal was to get x.as_integer_ratio() to run without type testing on all concrete numeric types that could support it.

    Serhiy is proposing to instead add a math.as_integer_ratio() function that would extract the components differently for different types (see bpo-37822). If that had been our plan, then it was a mistake to add as_integer_ratio() to int and bool (as they already have numerator and denominator attributes). If you change you mind and want to go with 37822, we should probably rip-out the 3.8 addition to int and bool.

    Jeroen is proposing to down yet another path for this (see bpo-28716). He wants to add a __ratio__ method to all the classes and access them with an operator.ratio() function. He thinks this will help the SageMath package.

    My recommendation is to stick with the original plan of adding the as_integer_ratio() to all the concrete types. The only one left to be done is in the Fractions module. That is pretty easy -- see PR 15212.

    There is some question about whether to change numbers.Rational() but that discussion can be left for another day -- concrete classes are allowed to have extra methods that aren't in the abstract classes. The abstract classes are harder to change because any existing user classes that had registered as Rational would instantly be non-compliant (though the fix is easy).

    I would like to get this finished up for 3.8. It doesn't make sense to me to have as_integer_ratio() for bool, int, float, and Decimal but to have omitted the most obvious case, Fraction. That defeats the purpose of having parallel APIs.

    @gvanrossum
    Copy link
    Member

    Let's continue on the current path -- add Fraction.as_integer_ratio().

    Note that as_integer_ratio() is not part of the Numbers API, it is an optional protocol.

    You can count me out for Jeroen's __ratio__ proposal.

    @rhettinger
    Copy link
    Contributor Author

    New changeset f03b4c8 by Raymond Hettinger in branch 'master':
    bpo-37819: Add Fraction.as_integer_ratio() (GH-15212)
    f03b4c8

    @rhettinger
    Copy link
    Contributor Author

    New changeset 5ba1cb0 by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
    bpo-37819: Add Fraction.as_integer_ratio() (GH-15212) (GH-15215)
    5ba1cb0

    @rhettinger
    Copy link
    Contributor Author

    Thanks.

    @serhiy-storchaka
    Copy link
    Member

    Sorry, but I do not understand why adding Fraction.as_integer_ratio() prevents adding math.as_integer_ratio().

    The user code can not benefit from this until we add as_integer_ratio() to all numeric numbers, and this is not realistic. For the same reason there is str.join() which works with arbitrary iterable instead of adding the join() method to all collections, iterators and generators.

    math.as_integer_ratio() makes the user code more general, clear and fast.

    @jdemeyer
    Copy link
    Contributor

    Sorry, but I do not understand why adding Fraction.as_integer_ratio() prevents adding math.as_integer_ratio().

    I also support a public function for that. It seems that we're planning this "as_integer_ratio" thing to become public API, so why not have a function as Serhiy proposes?

    I consider the situation with as_integer_ratio() very analogous to __index__ where we have operator.index(), so I would actually suggest operator.as_integer_ratio() but that's bikeshedding territory.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants