Implement fractional grouping in format specs - #8373
Conversation
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>
📝 WalkthroughWalkthroughFractional 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. ChangesFractional digit grouping
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 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
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_float.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:
|
There was a problem hiding this comment.
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
FormatSpecparsing to accept.[digits][,|_]and store a dedicatedfrac_grouping_option. - Apply fractional grouping during float/complex formatting, including correct width/zero-padding interactions.
- Update stdlib tests by removing
expectedFailuremarkers 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.
| _ => 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(()) |
There was a problem hiding this comment.
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'.
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (2)
Lib/test/test_float.pyis excluded by!Lib/**Lib/test/test_format.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/common/src/format.rscrates/vm/src/format.rs
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:The
,was never consumed byparse_precision, so it survived to thetrailing-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
FormatSpeccarries aseparate
frac_grouping_optionalongside the existing one — the samesplit CPython makes with
thousands_separators/frac_thousands_separator.Parsing follows
parse_internal_render_format_spec:https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L257-L299
,/_mix is rejected wherever it appears (.,_f,._,f)..,,f) is deliberately left in the spec. CPython'sthird check does not advance
pos, so the leftover reaches thetrailing-text check and produces a different message than the integer
part does for
,,— where only one character is left over, it becomesthe presentation type and the type/separator check reports it instead.
Format specifier missing precision(newFormatSpecError::PrecisionMissing)instead of an unrelated error. Previously
'{:.}'reported an unknownformat 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:
integer part has to reserve room for them, mirroring
calc_number_widths:https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L604-L606
'n'takes its separators from the locale and so cannot carry one:https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L361-L367
The complex locale path rewrites
'n'to'g'before delegating, so itnow validates before that rewrite. This also fixes a pre-existing gap:
format(1+2j, ',n')previously succeeded whileformat(1234.5, ',n')correctly failed.
Test Plan
Removed
@unittest.expectedFailurefrom tests that now pass(2 in
test_format, 1 intest_float). Thetest_floatone was nottargeted; it covers this option extensively, including the zero-padding
interaction above.
Added four unit tests in
crates/common/src/format.rscovering thegrouping 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
mainasthe baseline: 504,578 cases newly match CPython and none regressed.
The remaining mismatches are pre-existing and unrelated to this change:
Invalid format specifiernot carrying the spec and object type(tracked by
test_better_error_message_format), zero-padded complexreporting 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
Bug Fixes