cbor
cbor copied to clipboard
Add roundtrip example test case.
Hi, I was wondering whether this testcase should work or not? What am I doing wrong in this case, or is this a bug?
---- test_roundtrip stdout ----
thread 'test_roundtrip' panicked at 'called `Result::unwrap()` on an `Err` value: ErrorImpl { code: Message("invalid type: floating point `2.5`, expected f64"), offset: 0 }', src/libcore/result.rs:1165:5
The error message seems to indicate that we got a floating point, but we wanted a f64 value, this must be possible right?
This example serializes a data structure, and then again de-serializes the structure.
Btw, I tried serde_json
, and this crate works well with these lines:
let txt = serde_json::to_string(&stuff1).unwrap();
let stuff2 = serde_json::from_str(&txt).unwrap();
This is an instance of #144
Basically don't use flatten
until someone fixes this.
Any clue yet on how this should be fixed? I'm not into serde, so I do not yet grasp how it should work.
Also, the issue is more about the enum tag being improperly handled right? In this example, it's a field inside the flattened enum which behaves weird.
Should this be fixed in serde_cbor, or in serde itself?
Any clue yet on how this should be fixed?
No, no idea. That it works with json
is probably an indication that the problem lies within serde-cbor and not serde itself.
I found a solution to this issue. The solution was to always visit the f64 variant, since the cbor encoding is smart, and tries to use a compact value as possible, depending on the bits of the floating point number. This deserialization now starts to visit the f64 case (most bits), this will work onto f32 as well.
Please have a look at this, whether this makes sense or not.
Code fails to compile because of warnings.
I am not sure that this change will work as some code actually needs a f32 and not a f64. I recall that there was a similar issue in the past but I can't find the id right now.
I added a field in the roundtrip test case with of type f32 to test this. Is there a sort of stress test for rountripping all serde types available?
Excuse me for the many commits, please squash merge if merging this work.
@pyfisch what do you think about this change? Is it good / bad?
I had some second thoughts about the packing / unpacking logic of floating point values. I noticed that if a f64
value can be represented as a f32
or even and f16
value, this will be done during encoding.
I think this probably is counter intuitive in one of the following ways:
- Code on the decoding end (which may not be rust code) might expect the encoding to be an
f64
type. - Packet size will vary, subsequent packets vary in size. Not necessarily bad, but might be unexpected.
@windelbouwman Isn't using this patch use f64
rather than f16
or f32
, which increases unused size? What if there are f128
(#103), do we need to switch all of them to f128
too?
@pickfire I do not understand what you mean. Is there a f128
type? Then probably we need to use it as well.
That is in another pull request #103 like I mentioned.
@pickfire #103 is only for integer types, not floats. So there is no f128
.
@pyfisch what do you think of this change? Does it make sense?
As discussed in #179 serde-cbor will be rewritten and I don't want to maintain it in the future. Therefore I am no longer reviewing patches for new features and minor bugs. While this change fixes the problem you encounter other users may depend on an f32 and their code would be broken (I think).
Okay, thanks for the heads up!