GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Round float at the decimal level to match CPython's _Py_dg_dtoa by changjoon-park · Pull Request #7761 · RustPython/RustPython Conversation
|
| Probe set | Cases | Match |
|---|---|---|
| Targeted half-tie probes (positive ndigits) | 34 | 34/34 |
| Negative ndigits regression | 17 | 17/17 |
| Special values (NaN, ±Inf, ±0.0, NDIGITS bounds) | 12 | 12/12 |
No-arg round(x) (different code path) |
10 | 10/10 |
| Subnormal / denormal | 6 | 6/6 |
| Random fuzz (mantissa × ndigits ∈ {-5..50}) | 4332 | 4332/4332 |
Lib/test/test_float.py::RoundTestCase:
test_matches_float_format — rounds against float(format(x, '.{n}f')) across many values; this is exactly what the fix does internally.test_previous_round_bugs — explicit "round-half-even" cases (e.g. round(25.0, -1) == 20.0).cargo run --release -- -m test test_float test_builtin test_decimal \
test_fractions test_complex test_math test_format test_long test_int \
test_statistics test_re
1573 tests pass, 0 regressions. All 188 extra_tests/snippets/*.py pass under the CI feature set (stdlib,importlib,stdio,encodings,sqlite,ssl-rustls,host_env).
A regression block is added to extra_tests/snippets/builtin_round.py.
Bug Fixes
Tests
Sorry, something went wrong.
There was an error while loading. Please reload this page.
CPython's `float.__round__` (Objects/floatobject.c) routes through
`_Py_dg_dtoa` and rounds at the decimal level. The previous
`round_float_digits` multiplied by 10**ndigits and rounded at the
IEEE 754 binary level, which diverges for values that aren't exactly
representable. For example, 2.675 stores as 2.67499...; dtoa correctly
rounds it down to 2.67, but `(2.675 * 100.0).round() / 100.0` lands on
2.68 because the multiplication produces a phantom 267.5 tie that
round-half-to-even snaps up.
Rust's `{:.*}` float formatting uses dtoa-style algorithms (Grisu3 +
Dragon4 fallback) and matches CPython's `_Py_dg_dtoa` byte-for-byte.
Replace the multiply-then-round step with `format!` + `parse` for
ndigits >= 0. The ndigits < 0 path is unchanged because dividing
typical inputs by 10**|ndigits| produces genuine ties rather than
synthesizing them.
Verified byte-identical with CPython 3.14.4 over a 108-case random
fuzz plus targeted half-tie probes. Unmasks
`test_float.RoundTestCase.test_matches_float_format` and
`test_previous_round_bugs`.
There was an error while loading. Please reload this page.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRewrites ChangesFloat Rounding Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_float.py (TODO: 4) dependencies: dependent tests: (no tests depend on float) Legend:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm!
I had a minor nitpick
tysm for the patches:)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| let pow1 = 10.0f64.powi(-ndigits); | ||
| let y = x / pow1; | ||
| let z = y.round(); | ||
| #[allow(clippy::float_cmp)] |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| #[allow(clippy::float_cmp)] | |
| #[expect(clippy::float_cmp, reason = "...")] |
Can you please change it to expect and add a reason to this cfg?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
good catch i'll keep this convention following prs too thanks !
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Co-authored-by: ShaharNaveh <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
tysm:)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
thanks for the quick review !! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
* Round float at the decimal level to match CPython's _Py_dg_dtoa
CPython's `float.__round__` (Objects/floatobject.c) routes through
`_Py_dg_dtoa` and rounds at the decimal level. The previous
`round_float_digits` multiplied by 10**ndigits and rounded at the
IEEE 754 binary level, which diverges for values that aren't exactly
representable. For example, 2.675 stores as 2.67499...; dtoa correctly
rounds it down to 2.67, but `(2.675 * 100.0).round() / 100.0` lands on
2.68 because the multiplication produces a phantom 267.5 tie that
round-half-to-even snaps up.
Rust's `{:.*}` float formatting uses dtoa-style algorithms (Grisu3 +
Dragon4 fallback) and matches CPython's `_Py_dg_dtoa` byte-for-byte.
Replace the multiply-then-round step with `format!` + `parse` for
ndigits >= 0. The ndigits < 0 path is unchanged because dividing
typical inputs by 10**|ndigits| produces genuine ties rather than
synthesizing them.
Verified byte-identical with CPython 3.14.4 over a 108-case random
fuzz plus targeted half-tie probes. Unmasks
`test_float.RoundTestCase.test_matches_float_format` and
`test_previous_round_bugs`.
* Use #[expect] with reason for float_cmp suppression
Co-authored-by: ShaharNaveh <[email protected]>
---------
Co-authored-by: ShaharNaveh <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.