Skip to content

Conversation

@jordens
Copy link
Contributor

@jordens jordens commented Jul 27, 2022

Currently f{32,64}::NAN, INFINITE and NEG_INFINITY (i.e. if !v.is_finite()) are all emitted like their ryu serializations. This is wrong and breaks JSON.
Instead, emit null, like serde_json does.
On deserialization, interpret null as NAN where f32 or f64 is expected. This isn't exactly what serde_json does (it errors out instead), but makes the round-trip closer and somewhat more robust. And many would claim that it is common to interpret a JSON null as NAN where a float is expected (from schema/context/protocol).

jordens added 2 commits July 27, 2022 17:18
* they are not valid JSON numbers
* this behavior matches serde-json
* this is not what serde_json does (it errors instead)
* but it makes ser/de somewhat symmetric
Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

Sounds fine to me, thanks!
Could you add some tests and add the changes to the changelog?

@jordens jordens requested a review from eldruin August 8, 2022 13:23
Copy link
Member

@eldruin eldruin left a 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, thank you!

@eldruin eldruin merged commit bbe26b2 into rust-embedded-community:master Aug 8, 2022
@jordens jordens deleted the float-inf-nan branch August 8, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants