-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
seq: fix default float precision (%f
); fix 0 scientific printing
#7384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Retrigger CI with a force push, but I think the error count difference came from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR addresses inconsistencies in float precision handling and scientific notation formatting, and enables additional tests for verifying these behaviors.
- Update default float precision to 6 across all variants.
- Fix the capitalization of 0 in scientific notation and remove redundant code.
- Enable and add tests for default formatting for %f, %g, and %E formats.
Reviewed Changes
File | Description |
---|---|
src/uucore/src/lib/features/format/num_format.rs | Adjusts default precision and fixes scientific formatting for both zero and non-zero. |
tests/by-util/test_seq.rs | Enables and adds tests to verify default precision and formatting behavior. |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
GNU testsuite comparison:
|
GNU testsuite comparison:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
@@ -293,13 +293,7 @@ impl Formatter for Float { | |||
|
|||
let precision = match precision { | |||
Some(CanAsterisk::Fixed(x)) => x, | |||
None => { | |||
if matches!(variant, FloatVariant::Shortest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm good catch. I was wondering how this came to be and it seems to be this: 4c5326f
Looks like that commit only looked at %g
and check the other formats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did another rebase. Anything blocking this? (I have other changes on top of this that are starting to pile up...) |
GNU testsuite comparison:
|
0.0E+00 was not capitalized properly when using `%E` format. Fixes uutils#7382. Test: cargo test --package uucore --all-features float Test: cargo run printf "%E\n" 0 => 0.000000E+00
The default precision is 6, no matter the format. This applies to all float formats, not just "%g" (aka FloatVariant::Shortest). Fixes uutils#7361.
Add tests for some of the default float formats (%f, %g, %E), mostly to check that the default precision is correctly set to 6 digits.
Those tests appear to have been fixed, enable them.
GNU testsuite comparison:
|
Thanks ! |
The fact that printf and seq path are totally different is, TBH, a bit confusing...
printf uses precision 6 because of this subtle line in
spec.rs:Spec::write
(Self::Float
case):2 small additional changes (I can split to 2 other PR if needed, they felt small enough to just tack on this one):
%E
was wrong, fix that as well.seq: Enable test_auto_precision and test_undefined
Those tests appear to have been fixed, enable them.
seq: Add tests for default float formats
Add tests for some of the default float formats (%f, %g, %E), mostly
to check that the default precision is correctly set to 6 digits.
uucore: format: Fix default Float precision in try_from_spec
The default precision is 6, no matter the format. This applies
to all float formats, not just "%g" (aka FloatVariant::Shortest).
Fixes #7361.
uucore: format: Fix capitalization of 0 in scientific formating
0.0E+00 was not capitalized properly when using
%E
format.Fixes #7382.
Test: cargo test --package uucore --all-features float
Test: cargo run printf "%E\n" 0 => 0.000000E+00