Skip to content

Fix complex zero-padding error priority - #8453

Merged
youknowone merged 2 commits into
RustPython:mainfrom
shAn-kor:fix-complex-zero-padding-error-order
Aug 4, 2026
Merged

youknowone merged 2 commits into
RustPython:mainfrom
shAn-kor:fix-complex-zero-padding-error-order

Conversation

@shAn-kor

@shAn-kor shAn-kor commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

What changed

For complex values, 0 padding and = alignment are both invalid. When a format spec contains both, CPython raises the zero-padding error first. RustPython returned the alignment error in the regular formatting path, and the locale-aware n path applied the checks separately.

The validation now lives in one helper used by both paths. Specs such as =08.1f and =08n report the zero-padding error; =8.1f and =8n still report the alignment error.

The regression tests cover both paths.

Tests

  • cargo test -p rustpython-common format_complex_
  • cargo run --release -- -m test test_complex
  • cargo clippy -p rustpython-common --all-targets -- -D warnings
  • cargo fmt --all -- --check

AI assistance

Codex (GPT-5) assisted with comparing CPython and RustPython behavior, drafting the implementation and regression tests, and writing this PR description.

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation for complex number formatting options.
    • Consistently rejects unsupported combinations of zero-padding and after-sign alignment in standard and locale-aware formatting.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 11e965c8-cde4-431d-ad17-c2af0c5ac6bf

📥 Commits

Reviewing files that changed from the base of the PR and between 184db11 and 3811409.

📒 Files selected for processing (1)
  • crates/common/src/format.rs

📝 Walkthrough

Walkthrough

The change extracts shared validation for complex number padding and alignment. Standard and locale-aware formatting now use the same checks. New tests cover invalid zero-padding and after-sign alignment specifications.

Changes

Complex format validation

Layer / File(s) Summary
Shared padding and alignment validation
crates/common/src/format.rs
A private validator rejects zero-padding and after-sign alignment. Both complex formatting paths call the validator.
Standard and locale validation tests
crates/common/src/format.rs
Tests verify ZeroPadding and AlignmentFlag errors for standard and locale-aware complex formatting.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: z-ca-2026

Suggested reviewers: shaharnaveh, youknowone

🚥 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 describes the main change: correcting error priority for zero-padding in complex-number formatting.
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.

@shAn-kor
shAn-kor marked this pull request as ready for review August 4, 2026 10:08

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

👍 Thank you!

@youknowone
youknowone merged commit 6eea4fd into RustPython:main Aug 4, 2026
27 checks passed
@shAn-kor
shAn-kor deleted the fix-complex-zero-padding-error-order branch August 5, 2026 00:02
@coderabbitai coderabbitai Bot mentioned this pull request Aug 9, 2026
1 task done
youknowone pushed a commit that referenced this pull request Sep 16, 2026
@youknowone youknowone added the z-ca-2026 Tag to track Contribution Academy 2026 label Oct 5, 2026
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.

2 participants