8000 Support .as_integer_ratio() in fractions.Fraction · Issue #82017 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Support .as_integer_ratio() in fractions.Fraction #82017

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.

Sign up for GitHub

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

Jump to bottom
Closed
jdemeyer opened this issue Aug 13, 2019 · 9 comments
Closed

Support .as_integer_ratio() in fractions.Fraction #82017

jdemeyer opened this issue Aug 13, 2019 · 9 comments
Labels
3.14 bugs and security fixes stdlib Python modules in the Lib dir

Comments

@jdemeyer
Copy link
Contributor
jdemeyer commented Aug 13, 2019 •
edited by bedevere-app bot
Loading
BPO 37836
Nosy @rhettinger, @mdickinson, @serhiy-storchaka, @jdemeyer
PRs
  • bpo-37836: support .as_integer_ratio() in Fraction #15327
  • bpo-37836: document as_integer_ratio() in the data model #15328
    		•

    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 = None
created_at = <Date 2019-08-13.09:41:19.711>
labels = ['library', '3.9']
title = 'Support .as_integer_ratio() in fractions.Fraction'
updated_at = <Date 2022-01-02.22:00:12.998>
user = 'https://github.com/jdemeyer'

    bugs.python.org fields:

    activity = <Date 2022-01-02.22:00:12.998>
actor = 'rhettinger'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-08-13.09:41:19.711>
creator = 'jdemeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 37836
keywords = ['patch']
message_count = 6.0
messages = ['349536', '349933', '349940', '349942', '349945', '409531']
nosy_count = 5.0
nosy_names = ['rhettinger', 'mark.dickinson', 'stutzbach', 'serhiy.storchaka', 'jdemeyer']
pr_nums = ['15327', '15328']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue37836'
versions = ['Python 3.9']

    Linked PRs
    @jdemeyer
    Copy link
    Contributor Author

    Currently, the fractions.Fraction constructor accepts an .as_integer_ratio() method only for the specific types "float" and "Decimal". It would be good to support this for arbitrary classes.

    This is part of what was proposed in bpo-37822, 8000 but without adding the math.operator() function.

    @jdemeyer jdemeyer added 3.9 only security fixes stdlib Python modules in the Lib dir labels Aug 13, 2019
    @serhiy-storchaka
    Copy link
    Member

    I afraid this can slow down the Fraction constructor.

    @serhiy-storchaka
    Copy link
    Member

    See bpo-37884 which uses a C accelerator.

    @jdemeyer
    Copy link
    Contributor Author

    See bpo-37884 which uses a C accelerator.

    Note that that doesn't replace this issue, because I need to support as_integer_ratio both in the *numerator* and *denominator*.

    @jdemeyer
    Copy link
    Contributor Author

    I afraid this can slow down the Fraction constructor.

    No, it doesn't! It even speeds up the constructor in some cases:

    ./python -m perf timeit --duplicate 200 -s 'from fractions import Fraction; x = 1' 'Fraction(x)'
    BEFORE: Mean +- std dev: 826 ns +- 20 ns
    AFTER: Mean +- std dev: 814 ns +- 17 ns

    ./python -m perf timeit --duplicate 200 -s 'from fractions import Fraction; x = 1' 'Fraction(x, x)'
    BEFORE: Mean +- std dev: 1.44 us +- 0.03 us
    AFTER: Mean +- std dev: 1.46 us +- 0.02 us

    ./python -m perf timeit --duplicate 200 -s 'from fractions import Fraction; x = Fraction(1)' 'Fraction(x)'
    BEFORE: Mean +- std dev: 1.64 us +- 0.03 us
    AFTER: Mean +- std dev: 1.30 us +- 0.04 us

    ./python -m perf timeit --duplicate 200 -s 'from fractions import Fraction; x = Fraction(1)' 'Fraction(x, x)'
    BEFORE: Mean +- std dev: 3.03 us +- 0.05 us
    AFTER: Mean +- std dev: 2.34 us +- 0.06 us

    ./python -m perf timeit --duplicate 200 -s 'from fractions import Fraction; x = 1.0' 'Fraction(x)'
    BEFORE: Mean +- std dev: 1.82 us +- 0.02 us
    AFTER: Mean +- std dev: 1.29 us +- 0.04 us

    @rhettinger
    Copy link
    Contributor

    I don't think this is a door we should open:

       >>> Fraction(3.5, 2.5)
   Fraction(7, 5)

    This currently raises a useful exception:

    TypeError: both arguments should be Rational instances

    That is especially helpful in avoiding cases like this:

        >>> Fraction(1.1, 3.3)
    Fraction(2476979795053773, 7430939385161318)

    If that output is desired, the two conversions should be explicit.

        >>> Fraction(1.1) / Fraction(3.3)
    Fraction(2476979795053773, 7430939385161318

    I recommend rejecting this feature request as being more likely to be hazardous than helpful.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @serhiy-storchaka
    Copy link
    Member

    Concur with @rhettinger.

    @AA-Turner AA-Turner mentioned this issue Jun 8, 2022
    Closed
    This was referenced Mar 18, 2024
    Closed
    Closed
    @serhiy-storchaka
    Copy link
    Member

    This proposition was rejected because it used as_integer_ratio() in both single- and two-argument forms of the Fraction constructor. But these two forms have different roles. The former converts a number to a Fraction or parses a string representation of fraction. The latter creates a fraction from two rationals (and this is perhaps too much, it could be limited to integers). Conversion from integer supports not only rational numbers, but also float and Decimal. Supporting as_integer_ratio() here would allow to convert float128 and other non-rational numbers to Fraction.

    So I reopen this issue with more limited goal -- supporting as_integer_ratio() in a single-argument form of the Fraction constructor.

    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 8, 2024
    @serhiy-storchaka
    pythongh-82017: Support as_integer_ratio() in the Fraction constructor
    62bb662 
    Any numbers that have the as_integer_ratio() method (e.g. numpy.float128)
can now be converted to a fraction.
    @bedevere-app bedevere-app bot mentioned this issue Jun 8, 2024
    Merged
    @pfmoore
    Copy link
    Member
    pfmoore commented Jun 8, 2024

    Agreed this would be a nice generalisation for the case of converting a user-defined type to a fraction (the user defined type just has to define as_integer_ratio and then Fraction(value) works as expected). But it should not apply to the 2-argument case.

    @serhiy-storchaka serhiy-storchaka added 3.14 bugs and security fixes and removed 3.9 only security fixes labels Jul 17, 2024
    serhiy-storchaka added a commit that referenced this issue Jul 19, 2024
    @serhiy-storchaka
    gh-82017: Support as_integer_ratio() in the Fraction constructor (GH-…
    c8d2630 
    …120271)

Any objects that have the as_integer_ratio() method (e.g. numpy.float128)
can now be converted to a fraction.
    @InSyncWithFoo InSyncWithFoo mentioned this issue Nov 11, 2024
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.14 bugs and security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @pfmoore @rhettinger @jdemeyer @serhiy-storchaka
    0