gh-87790: support thousands separators for formatting fractional part of floats by skirpichev · Pull Request #125304 · python/cpython

skirpichev · 2024-10-11T07:52:41Z

>>> f"{123_456.123_456:_._f}"# Whole and fractional
'123_456.123_456'
>>> f"{123_456.123_456:_f}"# Integer component only
'123_456.123456'
>>> f"{123_456.123_456:._f}"# Fractional component only
'123456.123_456'
>>> f"{123_456.123_456:.4_f}"# with precision (._4f is illegal now)
'123456.123_5'
>>> f"{123_456.123_456:.4,f}"# with comma
'123456.123,5'

Notes for reviewers.

This is a simplest version, using _PyUnicode_InsertThousandsGrouping() like for the integer part. But perhaps we should insert digits in the fractional part, starting from the least significand digit instead, i.e. last example will print 123456.123_5 instead. Second commit puts separators, iterating from the least significand digit of fractional part.
The issue thread has several suggestions for grouping: e.g. 3 or 5. 3 was chosen.
Maybe grouping length should be configurable (e.g. ._3f - for grouping in three digits).
Heh, just for completeness: maybe break compatibility (see this)? I.e. _f will be "use underscore's both in the integer and the fractional part".
~~Also comma as a separator? (Perhaps, it's better to do in a separate pr.)~~

Issue: Add underscore as a decimal separator for string formatting #87790

📚 Documentation preview 📚: https://cpython-previews--125304.org.readthedocs.build/

…floats ```pycon >>> f"{123_456.123_456:_._f}" # Whole and fractional '123_456.123_456' >>> f"{123_456.123_456:_f}" # Integer component only '123_456.123456' >>> f"{123_456.123_456:._f}" # Fractional component only '123456.123_456' >>> f"{123_456.123_456:.4_f}" # with precision '123456.1_235' ```

rruuaanng

LGTM

skirpichev · 2024-11-16T09:59:33Z

(JFR, review request on d.p.o: https://discuss.python.org/t/71229)

nineteendo · 2024-11-17T13:37:01Z

 /* Determine the grouping, separator, and decimal point, if any. */
 if (get_locale_info(format->type == 'n' ? LT_CURRENT_LOCALE :
 format->thousands_separators,
+ format->frac_thousands_separator,


What are we going to do with the locale? At the very least I would reject specifying the separator manually:
>>>importlocale>>>locale.setlocale(locale.LC_ALL, 'af_za') >>>print(f"{123_456.123_456:.12,n}") 123.456,123,456# <- 2 decimal points
Would it be a breaking change to do this?
>>>print(f"{123_456.123_456:.12n}") 123.456,123.456

I think we should just reject separators with the n type.
Edit:
Would it be a breaking change to do this?
Yes, this will be a compatibility break.
IMO, without a legacy - it would be better to use thousands separators both for integer and fractional part. Then we could do same with n type formatting.
Edit2:
Maybe we should try something more closer to the original implementation? I.e. with a distinct optional frac_grouping option, which can have _ value. If present:
for floating-point format types that means: "apply current settings for thousands separators both to integer and fractional part";
for n type that means: "apply current locale settings for thousands separators both to integer and fractional part".
This is less flexible (you can't put separators to the fractional part only), but will more consistently interact with locale.

It's not possible to use the underscore separator with n format:
>>> format(1234, "_n") ValueError: Cannot specify '_' with 'n'.
So I agree that we should reject separators with the n type.

See invalid_thousands_separator_type() in parse_internal_render_format_spec().

@vstinner, this check already added. Unfortunately, that means with n type we can have locale-dependent separators only for integer part.

BTW, should we allow different separators? So far it's permitted:
>>> f"{122222.22222:_.7,f}" '122_222.222,220,0'

vstinner · 2024-11-18T13:28:17Z

 /* Determine the grouping, separator, and decimal point, if any. */
 if (get_locale_info(format->type == 'n' ? LT_CURRENT_LOCALE :
 format->thousands_separators,
+ format->frac_thousands_separator,


It's not possible to use the underscore separator with n format:
>>> format(1234, "_n") ValueError: Cannot specify '_' with 'n'.
So I agree that we should reject separators with the n type.

vstinner

LGTM. Thanks for the multiple updates.

vstinner · 2024-11-18T15:53:12Z

@serhiy-storchaka: Would you mind to review this change?

serhiy-storchaka · 2024-11-18T16:25:12Z


 .. productionlist:: format-spec
- format_spec: [[`fill`]`align`][`sign`]["z"]["#"]["0"][`width`][`grouping_option`]["." `precision`][`type`]
+ format_spec: [[`fill`]`align`][`sign`]["z"]["#"]["0"][`width`][`grouping_option`]["." `precision` [`grouping_option`]][`type`]


This is not accurate. More accurate would be ["." `precision` [`grouping_option`] | "." `grouping_option`] or ["." (`precision` [`grouping_option`] | `grouping_option`)] or ["." [`precision`][`grouping_option`]] with addition that either precision or grouping_option should be specified after ".".

How about this:
.. productionlist:: format-spec format_spec: [`options`][`width_and_precision`][`type`] options: [[`fill`]`align`][`sign`]["z"]["#"]["0"] fill: <any character> align: "<" | ">" | "=" | "^" sign: "+" | "-" | " " width_and_precision: [`width_with_grouping`][`precision_with_grouping`] width_with_grouping: [`width`][`grouping_option`] precision_with_grouping: "." (`precision`[`grouping_option`] | `grouping_option`) width: `~python-grammar:digit`+ grouping_option: "_" | "," precision: `~python-grammar:digit`+ type: "b" | "c" | "d" | "e" | "E" | "f" | "F" | "g" : | "G" | "n" | "o" | "s" | "x" | "X" | "%"
?

Redundant [...] in precision_with_grouping.

serhiy-storchaka · 2025-02-07T08:45:30Z

+ pos++;
+ }
+
+ while (pos<end && Py_ISDIGIT(PyUnicode_READ(kind, data, pos))) {


This should be in the if block.

Sorry, which if?

serhiy-storchaka

If this the way we will go, the implementation LGTM.

skirpichev · 2025-02-15T08:16:40Z

@vstinner, @serhiy-storchaka should we ask someone else to review this pr?

vstinner · 2025-02-25T15:27:21Z

I merged your PR, thanks.

bedevere-appBot mentioned this pull request Oct 11, 2024
Add underscore as a decimal separator for string formatting #87790
Closed

skirpichev force-pushed the fmt-frac-grouping-87790 branch from b648865 to 5858d8cCompare October 11, 2024 09:04

skirpichev marked this pull request as ready for review October 11, 2024 09:46

bedevere-appBot added the awaiting review label Oct 11, 2024

put separators in fractional part, iterating from most significand digit
d7b3784

rruuaanng reviewed Oct 12, 2024
View reviewed changes

Comment threadPython/formatter_unicode.c Outdated

rruuaanng reviewed Oct 12, 2024
View reviewed changes

Comment threadLib/test/test_float.py

address review: formatting
5b70fff

rruuaanng reviewed Oct 12, 2024
View reviewed changes

nineteendo reviewed Nov 15, 2024
View reviewed changes

Comment threadDoc/library/string.rst Outdated

skirpichev requested a review from nineteendo November 15, 2024 09:43

vstinner reviewed Nov 15, 2024
View reviewed changes

Comment threadPython/formatter_unicode.c Outdated

skirpichev added 2 commits November 16, 2024 05:56

Merge branch 'master' into fmt-frac-grouping-87790
42df636

address review: check return value of PyUnicode_FromOrdinal()
ca70d89

skirpichev changed the title ~~gh-87790: support underscore for formatting fractional part of floats~~gh-87790: support underscore/comma for formatting fractional part of floatsNov 16, 2024

skirpichev changed the title ~~gh-87790: support underscore/comma for formatting fractional part of floats~~gh-87790: support thousands separators for formatting fractional part of floatsNov 16, 2024

address review: support comma
28b9137

skirpichev requested a review from vstinner November 16, 2024 04:01

nineteendo reviewed Nov 16, 2024
View reviewed changes

Comment threadPython/formatter_unicode.c Outdated

skirpichev added 2 commits November 16, 2024 11:11

address review: fix error handling
0a0b3b0

add examples
59aab3c

skirpichev requested a review from nineteendo November 16, 2024 08:21

nineteendo reviewed Nov 16, 2024
View reviewed changes

Comment threadDoc/library/string.rst Outdated

address review: merge examples for comma/underscore
02d44de

skirpichev requested a review from nineteendo November 17, 2024 12:50

nineteendo reviewed Nov 17, 2024
View reviewed changes

skirpichev added 2 commits November 18, 2024 05:04

Merge branch 'master' into fmt-frac-grouping-87790
4d32cd5

address review: forbid separators for n
e359616

skirpichev requested a review from nineteendo November 18, 2024 03:11

vstinner reviewed Nov 18, 2024
View reviewed changes

nineteendo approved these changes Nov 18, 2024
View reviewed changes

bedevere-appBot added awaiting core review and removed awaiting review labels Nov 18, 2024

skirpichev added 2 commits November 18, 2024 18:10

address review: more tests
ea851c3

address review: add to whatsnew
4d3f0e3

vstinner approved these changes Nov 18, 2024
View reviewed changes

bedevere-appBot added awaiting merge and removed awaiting core review labels Nov 18, 2024

serhiy-storchaka approved these changes Nov 18, 2024
View reviewed changes

Merge branch 'main' into fmt-frac-grouping-87790
c008a6e

serhiy-storchaka reviewed Feb 7, 2025
View reviewed changes

serhiy-storchaka approved these changes Feb 7, 2025
View reviewed changes

skirpichev added 2 commits February 8, 2025 07:00

Merge branch 'master' into fmt-frac-grouping-87790
1c36219

address review: format-spec & more tests
893421d

vstinner merged commit f39a07b into python:mainFeb 25, 2025

bedevere-appBot removed the awaiting merge label Feb 25, 2025

skirpichev deleted the fmt-frac-grouping-87790 branch February 25, 2025 16:05

skirpichev mentioned this pull request May 23, 2025
Error in Format Specification Mini-Language concerning precision_with_grouping in 3.14 #134449
Closed

Uh oh!

Uh oh!

Conversation

skirpichev commented Oct 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rruuaanng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skirpichev commented Nov 16, 2024

Uh oh!

nineteendoNov 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skirpichevNov 18, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Nov 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skirpichevNov 19, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Feb 15, 2025

Uh oh!

vstinner commented Feb 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skirpichev commented Oct 11, 2024•
edited
Loading

nineteendoNov 17, 2024•
edited
Loading

skirpichevNov 18, 2024•
edited
Loading

skirpichevNov 19, 2024•
edited
Loading