| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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>
📝 Walkthrough
WalkthroughFractional 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: 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
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
📦 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:
|
Sorry, something went wrong.
There was a problem hiding this comment.
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:
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.
Sorry, something went wrong.
| _ => 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'.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify 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.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: cb1b3f70-706d-4c44-8b49-f9e7ca5b1ea6
📥 CommitsReviewing files that changed from the base of the PR and between 003ebec and 08dd625.
⛔ Files ignored due to path filters (2)
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good, thank you!
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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 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.
Parsing follows parse_internal_render_format_spec:
https://github.com/python/cpython/blob/3.14/Python/formatter_unicode.c#L257-L299
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.
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:
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
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 it
now validates before that rewrite. This also fixes a pre-existing gap:
format(1+2j, ',n') previously succeeded while format(1234.5, ',n')
correctly failed.
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
Bug Fixes