bpo-45995: add "z" format specifer to coerce negative 0 to zero by belm0 · Pull Request #30049 · python/cpython

belm0 · 2021-12-11T13:24:10Z

This option coerces negative zero into zero after rounding to the format precision.
Covers formatting of standard fraction types (float, Decimal, complex) via str.format(), built-in format(), and f-strings. Old-style string interpolation
is not supported.

https://www.python.org/dev/peps/pep-0682/

https://bugs.python.org/issue45995

belm0 · 2021-12-18T00:23:58Z

if PR #29438 lands, then detection of the "z" option can be incorporated into our forked mpd_parse_fmt_str_ex(), rather than this preprocessing of the format string

I think it's the case that one way or the other, the format spec is going to need to be completely parsed to figure out if a given 'z' is used for no_neg_0.

github-actions · 2022-01-18T00:06:52Z

This PR is stale because it has been open for 30 days with no activity.

mdickinson · 2022-01-22T10:02:18Z

Removing the stale label; discussion is ongoing on the bug tracker.

mdickinson · 2022-03-06T13:28:07Z

Update: the PEP has been accepted - https://discuss.python.org/t/accepting-pep-682-format-specifier-for-signed-zero/14088

We should aim to get this reviewed and in by the next (and final) alpha release of 3.11, due April 5th.

mdickinson

Hi @belm0. Here's a first-pass review; apologies for taking so long to get to this.

Most of the comments are nitpick-level. The big exception is the Decimal code, where I think there's still significant work to do.

bedevere-bot · 2022-03-13T11:55:02Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

mdickinson · 2022-03-13T11:57:51Z

@ericvsmith: if you have any bandwidth to look over the code at some point before the first 3.11 beta, I'd appreciate your input.

belm0

Thank you for the thorough review!

I'm bogged down a bit and it may take a week or so to reconcile the comments.

Hoping to get this merged for the next (final) alpha.

3.11.0 alpha 7: Tuesday, 2022-04-05
3.11.0 beta 1: Friday, 2022-05-06

This covers str.format() and f-strings. Old-style string interpolation is not supported. TODO: Decimal support

belm0 · 2022-03-23T23:19:34Z

+ if (no_neg_0 && mpd_isnegative(mpd) && !mpd_isspecial(mpd)) {
+ /* Round into a temporary (carefully mirroring the rounding
+ of mpd_qformat_spec()), and check if the result is negative zero.
+ If so, clear the sign and format the resulting positive zero. */


An insight I had recently was that, once negative zero is confirmed somehow, producing the correct output is only a matter of formatting positive zero (which cannot be affected by directed rounding, etc.).
As for how to detect negative zero, the options are: 1) round independently of the format function, or 2) analyze the string result of the format function (as Mark suggested). I found the former easier to manage, since it's a matter of duplicating a small amount of proven mpd library code, and is relatively straightforward to test. In contrast, analyzing the format output involves writing a new and perfectly-correct regex or parser, and imagining all the format spec edge cases to test it with.

mdickinson

Here's a second-pass review. We're getting there, but there are still a couple of bugs to resolve.

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>

mdickinson

Thank you for the updates! This looks ready to merge to me. I've been running some automated tests that cover a wide variety of combinations, and haven't yet managed to break this. :-)

A bit off-topic: I do think we're introducing some DRY-type technical debt into _decimal.c, though for this PR I don't see an easy alternative. For the future, there's scope for some cleanup after lifting the formatting machinery from libmpdec to _decimal.c. That's for another day, though.

ericvsmith

Other than possibly adding a credit in the blurb, this looks good to me.

ghost · 2022-04-11T11:51:03Z

Commit authors are required to sign the Contributor License Agreement.

belm0 · 2022-04-11T11:56:02Z

using file edit from the github UI appears to have angered CLA bot and bedevere

ericvsmith · 2022-04-11T11:56:58Z

using file edit from the github UI appears to have angered CLA bot and bedevere

I think it's related to the bpo to github issues migration. I'll see what I can figure out.

mdickinson · 2022-04-11T14:28:12Z

The "CLA Signing" check doesn't seem to be required for merging. Given that we know that CLA signing has occurred, I think we should be okay to merge. I've reported the failure to @ambv.

The implementation of PEP 682 was completed in python/cpython#30049.

belm0 requested a review from isidentical as a code owner December 11, 2021 13:24

the-knights-who-say-ni added the CLA signed label Dec 11, 2021

bedevere-bot added the awaiting review label Dec 11, 2021

belm0 marked this pull request as draft December 11, 2021 13:24

belm0 force-pushed the format_negative_zero branch from 97a033b to 8ff811fCompare December 11, 2021 13:44

belm0 marked this pull request as ready for review December 14, 2021 13:23

belm0 requested a review from rhettinger as a code owner December 14, 2021 13:23

belm0 commented Dec 18, 2021
View reviewed changes

Comment threadLib/_pydecimal.py Outdated

github-actionsBot added the stale Stale PR or inactive for long period of time. label Jan 18, 2022

taleinat requested a review from mdickinson January 18, 2022 07:38

mdickinson removed the stale Stale PR or inactive for long period of time. label Jan 22, 2022

belm0 force-pushed the format_negative_zero branch from 1bfb289 to bb4d5f1Compare February 4, 2022 02:46

mdickinson suggested changes Mar 13, 2022
View reviewed changes

bedevere-bot removed the awaiting review label Mar 13, 2022

bedevere-bot added the awaiting changes label Mar 13, 2022

ericvsmith reviewed Mar 13, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Library/2021-12-14-13-15-41.bpo-45995.Am9pNL.rst Outdated

belm0 commented Mar 15, 2022
View reviewed changes

Comment threadLib/test/test_float.py Outdated

belm0and others added 7 commits March 19, 2022 14:57

bpo-45995: add "z" format specifer to coerce negative 0 to zero
a633e88
This covers str.format() and f-strings. Old-style string interpolation is not supported. TODO: Decimal support

formatting
636da05

implementation for Decimal
3f4085d

📜🤖 Added by blurb_it.
68a049e

consistent flag names
ad32be5

add test case for integer value with z option
3dcaf5f

reference pending PEP
6b9ab3b

clarify Decimal strategy
2a24e61

belm0 commented Mar 23, 2022
View reviewed changes

mdickinson reviewed Apr 5, 2022
View reviewed changes

Comment threadLib/test/test_float.py
Comment threadPython/formatter_unicode.c Outdated
Comment threadModules/_decimal/_decimal.c Outdated
Comment threadModules/_decimal/_decimal.c Outdated
Comment threadModules/_decimal/_decimal.c

belm0and others added 3 commits April 7, 2022 09:14

fix Decimal format parsing
0cbff6a

fix Decimal when no precision is specified
8e7b51c

fix comment typo
418ab76
Co-authored-by: Mark Dickinson <dickinsm@gmail.com>

belm0 requested a review from mdickinson April 7, 2022 12:52

mdickinson approved these changes Apr 11, 2022
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Apr 11, 2022

ericvsmith reviewed Apr 11, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Library/2021-12-14-13-15-41.bpo-45995.Am9pNL.rst Outdated

ericvsmith approved these changes Apr 11, 2022
View reviewed changes

add attribution to news blurb
3ee6f6b

ericvsmith added skip issue CLA signed and removed CLA signed labels Apr 11, 2022

mdickinson merged commit b0b836b into python:mainApr 11, 2022

bedevere-bot removed the awaiting merge label Apr 11, 2022

mdickinson added a commit to mdickinson/peps that referenced this pull request Apr 11, 2022
Mark PEP 682 as final.
89fcfb2
The implementation of PEP 682 was completed in python/cpython#30049.

mdickinson mentioned this pull request Apr 11, 2022
PEP 682: Change status to Final python/peps#2513
Merged

belm0 deleted the format_negative_zero branch April 11, 2022 14:40

JelleZijlstra pushed a commit to python/peps that referenced this pull request Apr 11, 2022
PEP 682: Mark as final (#2513)
2c4d71f
The implementation of PEP 682 was completed in python/cpython#30049.

mdickinson mentioned this pull request Apr 11, 2022
string formatting: normalize negative zero #90153
Closed

navytux mentioned this pull request Apr 30, 2023
"z" format specifier is treated differently in unicode and bytes #104018
Closed

ericvsmith mentioned this pull request Jan 25, 2024
Possible memory leak in _cdecimal.c with 'z' format #114563
Closed

Uh oh!

Uh oh!

Conversation

belm0 commented Dec 11, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

belm0Dec 18, 2021

Choose a reason for hiding this comment

Uh oh!

ericvsmithMar 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actionsBot commented Jan 18, 2022

Uh oh!

mdickinson commented Jan 22, 2022

Uh oh!

mdickinson commented Mar 6, 2022

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Mar 13, 2022

Uh oh!

mdickinson commented Mar 13, 2022

Uh oh!

Uh oh!

belm0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

belm0Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ericvsmith left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Apr 11, 2022

Uh oh!

belm0 commented Apr 11, 2022

Uh oh!

ericvsmith commented Apr 11, 2022

Uh oh!

mdickinson commented Apr 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

belm0 commented Dec 11, 2021•
edited
Loading