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

gh-67790: Support float-style formatting for Fraction instances #100161

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Dec 10, 2022

This PR adds support for float-style formatting for Fraction objects: it supports the "e", "E", "f", "F", "g", "G" and "%" presentation types, and all the various bells and whistles of the formatting mini-language for those presentation types. The behaviour almost exactly matches that of float, but the implementation works with the exact Fraction value and does not do an intermediate conversion to float, and so avoids loss of precision or issues with numbers that are outside the dynamic range of the float type.

Note that the "n" presentation type is not supported. That support could be added later if people have a need for it.

There's one corner-case where the behaviour differs from that of float: for the float type, if explicit alignment is specified with a fill character of '0' and alignment type '=', then thousands separators (if specified) are inserted into the padding string:

>>> format(3.14, '0=11,.2f')
'0,000,003.14'

The exact same effect can be achieved by using the '0' flag:

>>> format(3.14, '011,.2f')
'0,000,003.14'

For Fraction, only the '0' flag has the above behaviour with respect to thousands separators: there's no special-casing of the particular '0=' fill-character/alignment combination. Instead, we treat the fill character '0' just like any other:

>>> format(Fraction('3.14'), '0=11,.2f')
'00000003.14'
>>> format(Fraction('3.14'), '011,.2f')
'0,000,003.14'

The Fraction formatter is also stricter about combining these two things: it's not permitted to use both the '0' flag and explicit alignment, on the basis that we should refuse the temptation to guess in the face of ambiguity. float is less picky:

>>> format(3.14, '0<011,.2f')
'3.140000000'
>>> format(Fraction('3.14'), '0<011,.2f')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mdickinson/Repositories/python/cpython/Lib/fractions.py", line 414, in __format__
    raise ValueError(
ValueError: Invalid format specifier '0<011,.2f' for object of type 'Fraction'; can't use explicit alignment when zero-padding

@netlify
Copy link

netlify bot commented Dec 10, 2022

Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit 983726f
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/6394be329cefee00083eddef

@mdickinson mdickinson added type-feature A feature request or enhancement 3.12 labels Dec 10, 2022
@mdickinson
Copy link
Member Author

mdickinson commented Dec 10, 2022

Converting to draft while I pacify the doc build.

@mdickinson mdickinson marked this pull request as draft Dec 10, 2022
@mdickinson
Copy link
Member Author

mdickinson commented Dec 10, 2022

Doc build duly pacified; ready for review.

@mdickinson mdickinson marked this pull request as ready for review Dec 10, 2022
@mdickinson
Copy link
Member Author

mdickinson commented Dec 14, 2022

@ericvsmith Would you be willing to review at some point? I'm not looking for detailed line-by-line review (though that would be useful too) so much as big-picture "is this a good idea?" review. In particular, I want to avoid doing anything here that will be hard to undo later if it conflicts with a different approach that we want to take, and that's why I restricted to just implementing the efg% presentation types, where the desired behaviour seems reasonably clear.

@ericvsmith
Copy link
Member

ericvsmith commented Dec 14, 2022

Hi, @mdickinson. Yes, I'll take a look. I'm going to be out of town for a few days, but will review when I get back.

@carljm carljm requested a review from ericvsmith Dec 14, 2022
@mdickinson
Copy link
Member Author

mdickinson commented Dec 15, 2022

@ericvsmith Thanks! No urgency - I'd quite like to get this in for 3.12, but we have a good few weeks (months, even!) before feature freeze.

@jowagner
Copy link

jowagner commented Dec 15, 2022

Can we also test the examples from the related issues:

  • '{:.2f}'.format(Fraction(1,3))
  • '{:.2f}'.format(Fraction(1,8))
  • '{:.2f}'.format(Fraction(3,8))
  • '{:.2f}'.format(Fraction(2545,1000))
  • '{:.2f}'.format(Fraction(2549,1000))
  • '{:.2f}'.format(Fraction(2635,1000))
  • '{:.1f}'.format(Fraction(1,100))
  • '{:.1f}'.format(Fraction(49,1000))
  • '{:.1f}'.format(Fraction(51,1000))
  • '{:.1f}'.format(Fraction(149,1000))
  • '{:.1f}'.format(Fraction(151,1000))

@mdickinson
Copy link
Member Author

mdickinson commented Dec 15, 2022

@jowagner Yes, I can definitely add those.

Lib/fractions.py Outdated Show resolved Hide resolved
Lib/fractions.py Outdated Show resolved Hide resolved
@mdickinson
Copy link
Member Author

mdickinson commented Dec 21, 2022

@jowagner

Can we also test the examples from the related issues:

Done in 4ccdf94.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 awaiting core review stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants