add deserialize impls for #2493 #2505
Conversation
- including proptest to verify roundtrip
| Ok(Value::Integer(int)) | ||
| } | ||
| "rational" => { | ||
| let parts = map.next_value::<Vec<serde_json::Value>>()?; |
There was a problem hiding this comment.
@bakaq having the value for "rational" be a list containing the numerator and the denominator made this somewhat difficult to deserialize here, due to one being signed while the other is unsigned and due to the fallback for large numbers.
There was a problem hiding this comment.
Can you think of any alternative? We need to store the numerator and denominator separately anyway, and being able to represent large integers is desirable.
There was a problem hiding this comment.
If we go with internally tagged we could just have a numerator and divisor field next to the type tag field i.e. { "type": "rational", numerator: ..., divisor: ... } or with untagged the value could be an object/map rather than a list i.e. { "rational": { "numerator": ..., "divisor": ... }}
There was a problem hiding this comment.
Oh, I like the internal tagging one.
Skgland
changed the title
| .prop_map(|map| QueryMatch { bindings: map }) | ||
| } | ||
|
|
||
| fn float_roundtrips(&orig: &'_ f64) -> bool { |
There was a problem hiding this comment.
We might want to enable the float_roundtrip feature for the serde_json crate to ensure all f64 roundtrip perfectly
No description provided.