-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PEP 786: Precision and Modulo-Precision Flag format specifiers for integer fields #4416
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
base: main
Are you sure you want to change the base?
Conversation
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.
I haven't read the content of the PEP yet, but a few notes:
It's better to give the formatspec one canonical ordering than permit an overly liberal rearrangeability. If commutativity were added, then as well as the messy description required for the docs for the particular case of `int` data, two people could write two different format spec that result in the same output and not realise it or agree, because they've written different things, leading to confusion etc.
I'll address the other ones in a separate commit(s) Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
…lutter in the format specification
I'm the author of this PEP. Sergey and Raymond shouldn't be pestered over it. They are in the appropriate 'Thanks' section.
Thank you to | ||
|
||
* Sergey B Kirpichev, for discussions and implementation code. | ||
* Raymond Hettinger, for the initial suggestion of the two's complement behavior. |
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.
Again, Raymond's initial suggestion was very different from proposed here: when value not fits to the two's complement format - error should be raised. In fact, it's now in "Rejected Alternatives", if I have read this huge text correctly.
I suggest you to either drop everything, related to specific handling of the z
flag, or ask @rhettinger if he is ok with proposed here behavior (i.e. z
will be an alias to value % 2**prec
for 'b'
type, no errors or anything else to indicate that value not fits).
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.
@rhettinger do you want to be included in the Thanks section? Your suggestion was part of the basis of the z
flag in this PEP.
The main difference is that we don't raise a ValueError
for 'out of range integers' since the modular arithmetic is more of a feature than a bug!
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.
I'll leave it in
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.
Sorry, it's not just about "thanks".
So far, your new handling of the 'z'
flag backed only by Raymond's feature request. But it's not clear if this issue was solved as author expected. Meanwhile, I suggest moving proposal for the 'z'
flag to the Open Issues section.
The new Abstract given by d477f4c seems sufficient.
No example needed for non-hardware aware modulo Remove the '[sic]' since we're not quoting Sergey.
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.
Disclaimer: My English, probably, is horrible. Perhaps, it was the reason why this text looks too difficult for me to follow and understand. Anyway, here my 2c as PEP reader.
The PEP text is huge, c.f. PEP 682 (roughly half of this). While it's idea (even with optional extension for 'z'
flag) looks damn simple. I think the text win if you address this issue.
Also, you might consider to better follow to standard section meanings. E.g. "Rationale" should describe why particular design decisions were made. While "Motivation" - why we need proposed features at all. "Specification", "Rejected Ideas", etc.
I also suggest to move 'z'
flag proposal to the "Open Issues" section. It was very briefly discussed in the ideas thread, not backed by existing feature requests and proposed specification clearly contradicts to the C behavior.
when formatting an integer with precision and one of the binary, octal, or | ||
hexadecimal presentation types (bases that are powers of two). This first | ||
reduces the integer into ``range(base ** precision)`` using the ``%`` operator. | ||
The result is a predictable two's complement style formatting with the *exact* |
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.
two's complement style formatting
What that means?
We endeavor to conclude in the following section that the former camp, lossless | ||
formatting, has no use cases, and is thus a rejected idea, whence this PEP |
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.
has no use cases
It's just not true. All your C examples demonstrate that. As well, as issue python/cpython#74756.
"Use case" is simple and obvious: to show two's complement value, not something "like" (that has no mathematical meaning).
value of ``-2 ** (n-1)``, to ``+2 ** (n-1)``, with a new ``n+1``\ th column of | ||
value ``-2 ** n`` prefixed on, the overall sum unaffected. | ||
|
||
This is what C's ``printf`` does, working with powers of two as the numbers of digits: |
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.
You might note, that "b" format type is available since the C23. (Works for me with -std=c2x
one recent gcc.)
New rebased PR with the correct branch name to avoid confusion (786 not 791).
Thank you Alyssa for sponsoring this!
Relevant discussions, issues, PRs linked
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approval: @ncoghlanAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate: Is a PEP-Delegate required?.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
📚 Documentation preview 📚: https://pep-previews--4416.org.readthedocs.build/pep-0786/