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

bedevere-bot · 2022-03-23T12:35:49Z

Thanks for making the requested changes!

@mdickinson: please review the changes made to this pull request.

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

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 Dec 11, 2021

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 Dec 11, 2021

belm0 force-pushed the format_negative_zero branch from 97a033b to 8ff811f Compare Dec 11, 2021

belm0 marked this pull request as ready for review Dec 14, 2021

belm0 requested a review from rhettinger as a code owner Dec 14, 2021

belm0 reviewed Dec 18, 2021

View changes

Lib/_pydecimal.py Show resolved Hide resolved

github-actions bot added the stale label Jan 18, 2022

taleinat requested a review from mdickinson Jan 18, 2022

mdickinson removed the stale label Jan 22, 2022

belm0 force-pushed the format_negative_zero branch from 1bfb289 to bb4d5f1 Compare Feb 4, 2022

mdickinson requested changes Mar 13, 2022

View 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 changes

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

belm0 reviewed Mar 15, 2022

View changes

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

belm0 and others added 7 commits Mar 19, 2022

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

bedevere-bot added the awaiting change review label Mar 23, 2022

bedevere-bot requested a review from mdickinson Mar 23, 2022

clarify Decimal strategy

2a24e61

belm0 reviewed Mar 23, 2022

View changes

mdickinson reviewed Apr 5, 2022

View 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 Apr 7, 2022

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 Apr 7, 2022

mdickinson approved these changes Apr 11, 2022

View changes

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

ericvsmith reviewed Apr 11, 2022

View 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 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
12 of 13 checks passed

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

mdickinson added a commit to mdickinson/peps that referenced this issue 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 Apr 11, 2022

JelleZijlstra pushed a commit to python/peps that referenced this issue 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

python / cpython Public

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

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

bedevere-bot commented Mar 23, 2022

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

python / cpython Public

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

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

bedevere-bot commented Mar 13, 2022

mdickinson commented Mar 13, 2022

belm0 left a comment

bedevere-bot commented Mar 23, 2022

belm0 Mar 23, 2022

Choose a reason for hiding this comment

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

belm0 commented Dec 11, 2021 •

edited