Skip to content

Implement fractional grouping in format specs - #8373

Merged
youknowone merged 1 commit into
RustPython:mainfrom
name-of-okja:feat/fractional-grouping-format-spec
Jul 26, 2026
Merged

Implement fractional grouping in format specs#8373
youknowone merged 1 commit into
RustPython:mainfrom
name-of-okja:feat/fractional-grouping-format-spec

Conversation

@name-of-okja

@name-of-okja name-of-okja commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

RustPython did not support the format spec syntax added in CPython 3.13
that places a grouping option after the precision, so anything of the
form .[digits][,|_] was rejected outright:

>>> format(1234.56789, '.6,f')
ValueError: Invalid format specifier          # CPython: '1234.567,890'

The , was never consumed by parse_precision, so it survived to the
trailing-text check at the end of FormatSpec::_parse.

This implements the option. The integer and fractional parts group
independently and may use different separators, so FormatSpec carries a
separate frac_grouping_option alongside the existing one — the same
split CPython makes with thousands_separators / frac_thousands_separator.

>>> format(1234.56789, '.6,f')      # fraction only
'1234.567,890'
>>> format(1234.56789, ',.6_f')     # both parts, different separators
'1,234.567_890'
>>> format(1.1, '.,f')              # precision omitted, type default applies
'1.100,000'

Parsing follows parse_internal_render_format_spec:
https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L257-L299

  • A ,/_ mix is rejected wherever it appears (.,_f, ._,f).
  • A repeated separator (.,,f) is deliberately left in the spec. CPython's
    third check does not advance pos, so the leftover reaches the
    trailing-text check and produces a different message than the integer
    part does for ,, — where only one character is left over, it becomes
    the presentation type and the type/separator check reports it instead.
  • A dot followed by neither digits nor a separator now raises
    Format specifier missing precision (new FormatSpecError::PrecisionMissing)
    instead of an unrelated error. Previously '{:.}' reported an unknown
    format code and '{:.f}' an invalid specifier.

Rendering groups the fraction digits away from the decimal point, so the
last group may be short and any exponent or percent tail is untouched.
The digit span is located the same way as CPython's parse_number:
https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L488-L516

Two details that are easy to miss:

Test Plan

  • Removed @unittest.expectedFailure from tests that now pass
    (2 in test_format, 1 in test_float). The test_float one was not
    targeted; it covers this option extensively, including the zero-padding
    interaction above.

  • Added four unit tests in crates/common/src/format.rs covering the
    grouping itself, the untouched tail, the width interaction, and the
    parse errors.

  • test_format, test_float, test_complex, test_int, test_fstring,
    test_types, test_str, test_decimal, test_locale, test_string:
    all pass, no unexpected successes.

  • Differentially compared against CPython 3.14 over ~900k generated format
    specs (alignment x sign x width/zero x integer separator x precision x
    type, across float/int/str/complex values), against a build of main as
    the baseline: 504,578 cases newly match CPython and none regressed.

    The remaining mismatches are pre-existing and unrelated to this change:
    Invalid format specifier not carrying the spec and object type
    (tracked by test_better_error_message_format), zero-padded complex
    reporting the alignment error instead of the zero-padding one, and a set
    separator with an unsupported presentation type reporting an unknown
    format code rather than Cannot specify ',' with 'j'..

Summary by CodeRabbit

  • New Features

    • Added support for grouping fractional digits with commas or underscores in numeric formatting.
    • Fractional grouping works with floating-point and complex values, including width calculations.
  • Bug Fixes

    • Improved validation and error messages for incomplete or invalid precision specifications.
    • Fractional grouping is now correctly rejected for locale-aware number formatting.

Support the CPython 3.13+ format spec syntax that puts a grouping option
after the precision, so `format(1234.56789, '.6,f')` gives
'1234.567,890'. The integer and fractional parts group independently and
may use different separators (`,.6_f` -> '1,234.567_890'), so the spec
carries a separate `frac_grouping_option`.

`parse_precision` now consumes the separator that follows the precision
digits and rejects a `,`/`_` mix. A repeated separator is deliberately
left in the spec so the trailing-text check reports it, which is how
CPython arrives at a different message there. A dot followed by neither
digits nor a separator now raises "Format specifier missing precision"
rather than an unrelated error.

Fraction digits group away from the decimal point, so the last group may
be short, and any exponent or percent tail is left intact. The
separators count toward the field width, so zero padding of the integer
part reserves room for them.

'n' takes its separators from the locale and so cannot carry one. The
complex locale path rewrites 'n' to 'g' before delegating, so it has to
validate first; that also makes `format(1+2j, ',n')` fail as it does in
CPython, which it previously did not.

Reference (CPython 3.14), Python/formatter_unicode.c:
- parsing: https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L257-L299
- 'n' rejection: https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L361-L367
- number split: https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L488-L516
- zero-pad width: https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L604-L606

Remove `@unittest.expectedFailure` from tests that now pass
(2 in test_format, 1 in test_float).

Assisted-by: Claude:claude-opus-5

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 26, 2026 00:42
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Fractional digit grouping is added to format specifications, including parsing, validation, float and complex formatting, width handling, and error conversion. Tests cover valid grouping, preserved suffixes, padding, and invalid combinations.

Changes

Fractional digit grouping

Layer / File(s) Summary
Parse fractional grouping and expose errors
crates/common/src/format.rs, crates/vm/src/format.rs
Format specifications parse comma or underscore fractional grouping, add PrecisionMissing, and convert it to ValueError.
Validate and apply fractional separators
crates/common/src/format.rs
Fractional separators are inserted into float and complex outputs, width calculations include them, and locale-aware n formatting rejects them.
Validate parsing and formatted output
crates/common/src/format.rs
Tests cover parsed fields, fractional grouping output, suffix preservation, width behavior, and validation errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: shaharnaveh, changjoon-park, youknowone

Sequence Diagram(s)

sequenceDiagram
  participant FormatSpec
  participant format_float
  participant add_frac_separators
  participant FormattedOutput
  FormatSpec->>format_float: provide fractional grouping option
  format_float->>add_frac_separators: group fractional digits
  add_frac_separators-->>format_float: preserve exponent and percent tails
  format_float-->>FormattedOutput: return padded formatted value
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding fractional grouping support to format specifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_float.py (TODO: 2)
[x] test: cpython/Lib/test/test_strtod.py (TODO: 2)

dependencies:

dependent tests: (no tests depend on float)

[x] test: cpython/Lib/test/test_format.py (TODO: 4)

dependencies:

dependent tests: (no tests depend on format)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR brings RustPython’s format-spec parsing and numeric rendering in line with CPython 3.13+ by supporting the fractional-grouping syntax placed after precision (.[digits][,|_]). It updates the core FormatSpec representation and rendering logic so integer and fractional grouping can be applied independently, and aligns error reporting for missing precision after ..

Changes:

  • Extend FormatSpec parsing to accept .[digits][,|_] and store a dedicated frac_grouping_option.
  • Apply fractional grouping during float/complex formatting, including correct width/zero-padding interactions.
  • Update stdlib tests by removing expectedFailure markers now that behavior matches CPython, and add unit tests for fractional grouping and new parse errors.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
Lib/test/test_format.py Removes expectedFailure decorators for mixed-separator error-message tests that now pass.
Lib/test/test_float.py Removes expectedFailure for float formatting tests now supported by RustPython.
crates/vm/src/format.rs Maps the new PrecisionMissing format error to a ValueError message.
crates/common/src/format.rs Implements fractional grouping parsing/storage and rendering, plus unit tests for the new behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 494 to +502
_ => Ok(()),
}?;
if let Some(grouping) = self.frac_grouping_option
&& matches!(format_type, FormatType::Number(_))
{
let ch = char::from(format_type);
return Err(FormatSpecError::UnspecifiedFormat(char::from(grouping), ch));
}
Ok(())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPython only rejects the fractional separator for the locale-aware n type, not for other presentation types.

The type/separator switch is guarded by if (format->thousands_separators), so it validates the integer-part separator only:

https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L331-L359

The fractional separator is validated separately, and that check tests format->type == 'n' and nothing else:

https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L361-L367

Verified against CPython 3.14.0b4:

>>> format('x', '.,s')
'x'
>>> format(1234, '.,d')
'1234'
>>> format(1234, '.,x')
'4d2'
>>> format(1234, '.,b')
'10011010010'

>>> format('x', ',s')
ValueError: Cannot specify ',' with 's'.
>>> format(1234, ',x')
ValueError: Cannot specify ',' with 'x'.

This PR matches all of those exactly — the integer-part cases are rejected by the existing grouping_option validation, and the fractional ones are accepted as CPython accepts them. Applying the same validation to frac_grouping_option would make format('x', '.,s') raise where CPython returns 'x'.

@moreal moreal added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 26, 2026
@name-of-okja
name-of-okja marked this pull request as ready for review July 26, 2026 01:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/common/src/format.rs`:
- Around line 496-502: Update validate_format() so frac_grouping_option rejects
all non-float presentation types, including s, b, c, d, o, x, X, %, and existing
n/N handling, by returning UnspecifiedFormat with the grouping and presentation
characters. Keep e, E, f, F, g, and G valid, and add regression tests covering
string and integer format specifications.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: cb1b3f70-706d-4c44-8b49-f9e7ca5b1ea6

📥 Commits

Reviewing files that changed from the base of the PR and between 003ebec and 08dd625.

⛔ Files ignored due to path filters (2)
  • Lib/test/test_float.py is excluded by !Lib/**
  • Lib/test/test_format.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/common/src/format.rs
  • crates/vm/src/format.rs

Comment thread crates/common/src/format.rs

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@youknowone
youknowone merged commit 9add522 into RustPython:main Jul 26, 2026
27 checks passed
@name-of-okja
name-of-okja deleted the feat/fractional-grouping-format-spec branch August 8, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants