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

Jump to bottom

Support PEP 515 for Fraction's initialization from string #88424

Closed
skirpichev mannequin opened this issue May 28, 2021 · 6 comments
Closed

Support PEP 515 for Fraction's initialization from string #88424

skirpichev mannequin opened this issue May 28, 2021 · 6 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@skirpichev
Copy link
Mannequin

skirpichev mannequin commented May 28, 2021

BPO 44258
Nosy @mdickinson, @vedgar, @skirpichev
PRs
  • bpo-44258: support PEP 515 for Fraction's initialization from string #26422
    		•
    Files
  • fractions-from-str.diff
    		•

    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 2021-06-07.07:07:33.971>
created_at = <Date 2021-05-28.05:47:31.699>
labels = ['type-feature', 'library', '3.11']
title = "Support PEP 515 for Fraction's initialization from string"
updated_at = <Date 2021-06-07.07:07:33.970>
user = 'https://github.com/skirpichev'

    bugs.python.org fields:

    activity = <Date 2021-06-07.07:07:33.970>
actor = 'mark.dickinson'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-07.07:07:33.971>
closer = 'mark.dickinson'
components = ['Library (Lib)']
creation = <Date 2021-05-28.05:47:31.699>
creator = 'Sergey.Kirpichev'
dependencies = []
files = ['50069']
hgrepos = []
issue_num = 44258
keywords = ['patch']
message_count = 6.0
messages = ['394633', '394638', '394642', '394730', '394731', '395249']
nosy_count = 3.0
nosy_names = ['mark.dickinson', 'veky', 'Sergey.Kirpichev']
pr_nums = ['26422']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44258'
versions = ['Python 3.11']
    @skirpichev
    Copy link
    Mannequin Author

    skirpichev mannequin commented May 28, 2021 

    Right now:
>>> from fractions import Fraction as F
>>> F(1_2_3, 3_2_1)
Fraction(41, 107)

    but

    >>> F('1_2_3/3_2_1')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sk/src/cpython/Lib/fractions.py", line 115, in __new__
    raise ValueError('Invalid literal for Fraction: %r' %
ValueError: Invalid literal for Fraction: '1_2_3/3_2_1'

or even this (should be consistent with int constructor, isn't?):
>>> F('1_2_3')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sk/src/cpython/Lib/fractions.py", line 115, in __new__
    raise ValueError('Invalid literal for Fraction: %r' %
ValueError: Invalid literal for Fraction: '1_2_3'

    Tentative patch attached. Let me know if this does make sense as a PR.

    @skirpichev skirpichev mannequin added 3.11 only security fixes stdlib Python modules in the Lib dir labels May 28, 2021
    @mdickinson
    Copy link
    Member

    +1 to the idea: Decimal, float, complex and int all understand the underscores; there's no good reason for Fraction not to.

    So yes please to the PR. There should be tests that check that only the underscore rules allowed by int and the others are supported: e.g., Fraction("1__2"), and Fraction("1_2_") should be errors.

    @skirpichev
    Copy link
    Mannequin Author

    skirpichev mannequin commented May 28, 2021

    On Fri, May 28, 2021 at 06:48:14AM +0000, Mark Dickinson wrote:

    So yes please to the PR. There should be tests that check that only the
    underscore rules allowed by int and the others are supported: e.g.,
    Fraction("1__2"), and Fraction("1_2_") should be errors.

    Ok, I did.

    In the initial version I catch int()'s exceptions to
    return a correct exception details. Maybe it's better to fix
    the regexp instead (don't match "wrong" strings). I think
    this might be more complicated for readers...

    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented May 29, 2021

    How about '1_/_2'? I think making / more separated adds value... though of course, someone will ask about '1 / 2' next. :-)

    @mdickinson
    Copy link
    Member

    How about '1_/_2'?

    I'd rather keep it consistent with the rules for int: that is, if I split on '/', I'd expect the pieces to be parseable by int.

    As for spaces around the /, let's make that a separate issue.

    @mdickinson
    Copy link
    Member

    New changeset 89e50ab by Sergey B Kirpichev in branch 'main':
    bpo-44258: support PEP-515 for Fraction's initialization from string (GH-26422)
    89e50ab

    @mdickinson mdickinson added the type-feature A feature request or enhancement label Jun 7, 2021
    @mdickinson mdickinson added the type-feature A feature request or enhancement label Jun 7, 2021
    @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
    Assignees
    No one assigned
    Labels
    3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @mdickinson