Skip to content
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

Only format Unexpected::Float with decimal point if it is finite #2733

Merged
merged 1 commit into from
May 1, 2024

Conversation

jamessan
Copy link
Contributor

@jamessan jamessan commented May 1, 2024

bef110b changed the display for unexpected floats to always append a
".0" if there was no decimal point found in the formatting of the float.

However, this should only be relevant for finite (i.e., not NaN or inf)
values. The change introduced a test failure in the ordered-float
crate due to this:

 ---- impl_serde::test_fail_on_nan stdout ----
 thread 'impl_serde::test_fail_on_nan' panicked at 'assertion failed: `(left == right)`
   left: `Error { msg: "invalid value: floating point `NaN.0`, expected float (but not NaN)" }`,
  right: `"invalid value: floating point `NaN`, expected float (but not NaN)"`', src/lib.rs:1554:9
 stack backtrace:
    0: rust_begin_unwind
              at /usr/src/rustc-1.70.0/library/std/src/panicking.rs:578:5
    1: core::panicking::panic_fmt
              at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:67:14
    2: core::panicking::assert_failed_inner
    3: core::panicking::assert_failed
              at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:228:5
    4: serde_test::assert::assert_de_tokens_error
              at /usr/share/cargo/registry/serde_test-1.0.171/src/assert.rs:228:19
    5: ordered_float::impl_serde::test_fail_on_nan
              at ./src/lib.rs:1554:9
    6: ordered_float::impl_serde::test_fail_on_nan::{{closure}}
              at ./src/lib.rs:1553:27
    7: core::ops::function::FnOnce::call_once
              at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
    8: core::ops::function::FnOnce::call_once
              at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

bef110b changed the display for unexpected floats to always append a
".0" if there was no decimal point found in the formatting of the float.

However, this should only be relevant for finite (i.e., not NaN or inf)
values.  The change introduced a test failure in the ordered-float
crate due to this:

     ---- impl_serde::test_fail_on_nan stdout ----
     thread 'impl_serde::test_fail_on_nan' panicked at 'assertion failed: `(left == right)`
       left: `Error { msg: "invalid value: floating point `NaN.0`, expected float (but not NaN)" }`,
      right: `"invalid value: floating point `NaN`, expected float (but not NaN)"`', src/lib.rs:1554:9
     stack backtrace:
        0: rust_begin_unwind
                  at /usr/src/rustc-1.70.0/library/std/src/panicking.rs:578:5
        1: core::panicking::panic_fmt
                  at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:67:14
        2: core::panicking::assert_failed_inner
        3: core::panicking::assert_failed
                  at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:228:5
        4: serde_test::assert::assert_de_tokens_error
                  at /usr/share/cargo/registry/serde_test-1.0.171/src/assert.rs:228:19
        5: ordered_float::impl_serde::test_fail_on_nan
                  at ./src/lib.rs:1554:9
        6: ordered_float::impl_serde::test_fail_on_nan::{{closure}}
                  at ./src/lib.rs:1553:27
        7: core::ops::function::FnOnce::call_once
                  at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
        8: core::ops::function::FnOnce::call_once
                  at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
     note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit 2d973c1 into serde-rs:master May 1, 2024
15 checks passed
@jamessan jamessan deleted the nan-decimal branch May 1, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants