-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
Conversation
✅ Deploy Preview for python-cpython-preview canceled.
|
Converting to draft while I pacify the doc build. |
Doc build duly pacified; ready for review. |
… 'e' presentation type
@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 |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @mdickinson. Sorry to take so long to review this, I've been unusually busy.
The code looks reasonable enough to me. I didn't go over it with a fine-toothed comb, but none the less I didn't see anything odd.
My only comment would be: what if you want to format the number as a fraction, but with the integer parts formatted? That is, if you wanted "52,163/16,604" or the less likely "0xcbc3/0x40dc"? I realize that since you only are supporting the floating point presentation types, anything else could be added in the future. So I guess as long as we reserve the integer presentation types ('b', 'd', 'o', 'x', 'X'), nothing precludes them being used in the future.
In which case I suppose it would be like complex
: the formatting would be applied to both the numerator and denominator, and weird things happen with the width (in complex.__format__
I think any precision and width applies to both real and imaginary parts: I'm not sure this is the best decision I ever made, but then again I can't imagine how else to apply the width value).
I forgot to add: yes, I think this is a good idea! |
@ericvsmith Thanks for reviewing! I'll do another round of self-review, and merge shortly if I don't spot anything blocking.
Yes, that was where I was going. Of those, I'm finding it hard to imagine use-cases for anything other than presentation type
|
I think I'm missing something here. Precision doesn't apply, of course, so it's only the width we need to worry about. I was imagining that this would simply be a minimum width, applied to the formatted fraction as a whole. So for example: >>> f"{Fraction(22, 7):}"
'22/7'
>>> f"{Fraction(22, 7):10}" # or '10d' in place of '10'
' 22/7' I guess that would mean that the slashes wouldn't be nicely aligned in a table of fractions, but I think I could live with that - if anyone needs that alignment they could format the numerator and denominator separately: >>> f = Fraction(22, 7)
>>> f"{f.numerator:>5}/{f.denominator:<5}"
' 22/7 ' We could try to get fancy and re-use the precision as a minimum width for the denominator: >>> f"{Fraction(22, 7):10.5}"
' 22/ 7' But this feels like a bit of an abuse, and it also feels as though we're getting into YAGNI territory, and that we're guessing about use-cases without information from actual users. |
@ericvsmith I've made a proof-of-concept PR (against the branch for this PR) for adding support for the 'd' presentation type at mdickinson#2. |
I think you're right in applying the width to the whole fraction, including the slash. No need to go nutso on this, and as you say, the users could format the numerator and denominator separately. I'll take a look at your branch. |
@ericvsmith Thanks. I'll merge this one. There were a few inconsequential style / cosmetic updates since your review, and one bugfix + regression test ("Z" was being accepted as the suppress-negative-zero flag, where only "z" should have been allowed). |
For what it's worth, I spent about a day implementing a tabular format for fractions that aligned the slashes, and I threw the whole thing out because left-aligning the denominators made the fractions too hard to read. Alignment around a radix point works great because it keeps all of the tens, units, tenths, etc. in vertical alignment. The same isn't true for fraction slashes (unless you normalize everything to the same denominator) so it just makes the rows ragged and harder to scan. |
https://build.opensuse.org/request/show/1073017 by user dgarcia + dimstar_suse - Enable python 3.11 build again, now is supported - Update to 1.14 - Implement __format__ for Fraction, following python/cpython#100161 - Implement Fraction.is_integer(), following python/cpython#100488 - Fraction.limit_denominator() is faster, following python/cpython#93730 - Internal creation of result Fractions is about 10% faster if the calculated numerator/denominator pair is already normalised, following python/cpython#101780 - Built using Cython 3.0.0b1. - 1.13 - Parsing very long numbers from a fraction string was very slow, even slower than fractions.Fraction. The parser is now faster in all cases (and still much faster for shorter numbers). - Fraction did not implement __int__. https://bugs.python.org/issue44547 - 1.12 - Faster and more spa
python 3.12 supports float-style formatting for Fraction by python/cpython#100161 . With this change, when ":n" format specifier is used in format() for Fraction type, this now raises ValueError instead of previous TypeError. To make pytest succeed with python 3.12, make pint.testing.assert_allclose also rescue ValueError . Fixes hgrecco#1818 .
PR #100161 added fancy float-style formatting for the Fraction type, but left us in a state where basic formatting for fractions (alignment, fill, minimum width, thousands separators) still wasn't supported. This PR adds that support. --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
PR python#100161 added fancy float-style formatting for the Fraction type, but left us in a state where basic formatting for fractions (alignment, fill, minimum width, thousands separators) still wasn't supported. This PR adds that support. --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
PR python#100161 added fancy float-style formatting for the Fraction type, but left us in a state where basic formatting for fractions (alignment, fill, minimum width, thousands separators) still wasn't supported. This PR adds that support. --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
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 offloat
, but the implementation works with the exactFraction
value and does not do an intermediate conversion tofloat
, and so avoids loss of precision or issues with numbers that are outside the dynamic range of thefloat
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:The exact same effect can be achieved by using the
'0'
flag: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: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: