bpo-45995: add "z" format specifer to coerce negative 0 to zero #30049

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

Modules/_decimal/_decimal.c

+        char *z_start = strchr(fmt, 'z');
+        if (z_start != NULL) {
+            no_neg_0 = 1;
+            size_t z_index = z_start - fmt;
+            if (fmt_copy == NULL) {
+                fmt = fmt_copy = dec_strdup(fmt, size);
+                if (fmt_copy == NULL) {
+                    return NULL;
+                }
+            }
+            memmove(fmt_copy + z_index, fmt_copy + z_index + 1, size - z_index);
+            size -= 1;


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.

Lib/_pydecimal.py

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.

Doc/library/string.rst

Lib/_pydecimal.py

Lib/pydoc_data/topics.py

Lib/test/test_decimal.py

Modules/_decimal/_decimal.c

Objects/bytesobject.c

Python/pystrtod.c

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.

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

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

Lib/test/test_float.py

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

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

Modules/_decimal/_decimal.c

+    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.

Lib/test/test_float.py

Python/formatter_unicode.c

Modules/_decimal/_decimal.c

Co-authored-by: Mark Dickinson <[email protected]>

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.

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

ericvsmith

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

cpython-cla-bot · 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 8ff811f Compare 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

Lib/_pydecimal.py Show resolved Hide resolved

github-actions bot 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 bb4d5f1 Compare 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

Misc/NEWS.d/next/Library/2021-12-14-13-15-41.bpo-45995.Am9pNL.rst Outdated Show resolved Hide resolved

belm0 commented Mar 15, 2022

View reviewed changes

Lib/test/test_float.py Outdated Show resolved Hide resolved

belm0 and 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

Lib/test/test_float.py Show resolved Hide resolved

Python/formatter_unicode.c Outdated Show resolved Hide resolved

Modules/_decimal/_decimal.c Outdated Show resolved Hide resolved

Modules/_decimal/_decimal.c Show resolved Hide resolved

Modules/_decimal/_decimal.c Show resolved Hide resolved

belm0 and 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 <[email protected]>

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

Misc/NEWS.d/next/Library/2021-12-14-13-15-41.bpo-45995.Am9pNL.rst Outdated Show resolved Hide resolved

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:main Apr 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-45995: add "z" format specifer to coerce negative 0 to zero #30049

bpo-45995: add "z" format specifer to coerce negative 0 to zero #30049

belm0 commented Dec 11, 2021 •

edited

Loading

belm0 Dec 18, 2021

ericvsmith Mar 15, 2022

github-actions bot commented Jan 18, 2022

mdickinson commented Jan 22, 2022

mdickinson commented Mar 6, 2022

mdickinson left a comment

bedevere-bot commented Mar 13, 2022

mdickinson commented Mar 13, 2022

belm0 left a comment

belm0 Mar 23, 2022

mdickinson left a comment

mdickinson left a comment

ericvsmith left a comment

cpython-cla-bot bot commented Apr 11, 2022

belm0 commented Apr 11, 2022

ericvsmith commented Apr 11, 2022

mdickinson commented Apr 11, 2022

bpo-45995: add "z" format specifer to coerce negative 0 to zero #30049

bpo-45995: add "z" format specifer to coerce negative 0 to zero #30049

Conversation

belm0 commented Dec 11, 2021 • edited Loading

belm0 Dec 18, 2021

Choose a reason for hiding this comment

ericvsmith Mar 15, 2022

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2022

mdickinson commented Jan 22, 2022

mdickinson commented Mar 6, 2022

mdickinson left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 13, 2022

mdickinson commented Mar 13, 2022

belm0 left a comment

Choose a reason for hiding this comment

belm0 Mar 23, 2022

Choose a reason for hiding this comment

mdickinson left a comment

Choose a reason for hiding this comment

mdickinson left a comment

Choose a reason for hiding this comment

ericvsmith left a comment

Choose a reason for hiding this comment

cpython-cla-bot bot commented Apr 11, 2022

belm0 commented Apr 11, 2022

ericvsmith commented Apr 11, 2022

mdickinson commented Apr 11, 2022

belm0 commented Dec 11, 2021 •

edited

Loading