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

Roundtrip using "serde-float" and "serde-arbitrary-precision" fails #402

Closed
JamesHinshelwood opened this issue Jul 15, 2021 · 4 comments · Fixed by #404
Closed

Roundtrip using "serde-float" and "serde-arbitrary-precision" fails #402

JamesHinshelwood opened this issue Jul 15, 2021 · 4 comments · Fixed by #404

Comments

@JamesHinshelwood
Copy link
Contributor

The assertion in the following code fails, whereas I would expect it to succeed based on the features I have activated:

main.rs:

use rust_decimal::Decimal;

fn main() {
    let dec = Decimal::new(481, 2);
    let serialized = serde_json::to_value(dec).unwrap();
    let dec2: Decimal = serde_json::from_value(serialized).unwrap();

    assert_eq!(dec, dec2);
}

Cargo.toml:

[package]
name = "rust_decimal_test"
version = "0.1.0"
authors = ["..."]
edition = "2018"

[dependencies]
rust_decimal = { version = "1.14.3", features = ["serde-float", "serde-arbitrary-precision"]}
serde_json = { version = "1.0.64", features = ["arbitrary_precision"] }

Output:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `4.81`,
 right: `4.8100000000000005`', src/main.rs:8:5

Reading the documentation of serde-arbitrary-precision further, it makes it clear that it only supports arbitrary precision when parsing decimals but not when serializing them. .to_f64() is still used in the Serialize impl which causes the rounding issue.

Would you be opposed to extending the behaviour of the serde-arbitrary-precision feature to also serialize Decimals to JSON numbers exactly, so that the above test would pass?

I'd be willing to attempt to contribute this change if you thought it was a good idea.

@JamesHinshelwood
Copy link
Contributor Author

I've raised a #404 for an attempt at fixing this. Let me know what you think!

@paupino
Copy link
Owner

paupino commented Jul 15, 2021

Hi @JamesHinshelwood,
Thanks for issue write up and for publishing the PR!
I think that this area of the library is actually quite confusing and could do with some clearer documentation/examples - perhaps later flattening features. Anyway, can you try one thing for me? Can you remove the serde-float feature and see if that solves your issues? e.g.

[package]
name = "rust_decimal_test"
version = "0.1.0"
authors = ["..."]
edition = "2018"

[dependencies]
rust_decimal = { version = "1.14.3", features = ["serde-arbitrary-precision"]}
serde_json = { version = "1.0.64", features = ["arbitrary_precision"] }

Or is your issue with the format that it is being serialized to when using serde-arbitrary-precision (hence also using serde-float)?

@JamesHinshelwood
Copy link
Contributor Author

@paupino Thanks for the response.

You're right. Removing the serde-float feature means the test passes, however that means Decimals get serialized to serde_json::Value::Strings rather than serde_json::Value::Numbers as I wanted. In fact, in this case I think there's no need for the arbitrary_precision features either, since Decimals can be serialized and deserialized to Strings exactly already.

@joseph-wakeling-frequenz

@JamesHinshelwood @paupino hope you don't mind me dropping in on this (closed!) issue, as I have some similar use-cases.

However, note that the String solution is valid only if it is acceptable to the other party in the data exchange. If one has a use-case where a 3rd-party API expects values in JSON number format (and only in number format), it seems important to be able to (de)serialize decimal values precisely.

Any chance that the issue could be re-awakened with this in mind?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants