feat(spec): Expose DataFile serde functions #1563
Conversation
CTTY
changed the title
| "referenced_data_file": null, | ||
| "content_offset": null, | ||
| "content_size_in_bytes": null | ||
| }); |
There was a problem hiding this comment.
After some tinkering, I think using serde_json::Value to check the json output gives the most readable test.
Also I found a tricky corner case: DataFile's serializer won't preserve the order of DataFile's array like column_sizes. Right now I just make the array sizes to be one to work around that, maybe there is a better solution?
There was a problem hiding this comment.
Also I found a tricky corner case: DataFile's serializer won't preserve the order of DataFile's array like column_sizes. Right now I just make the array sizes to be one to work around that, maybe there is a better solution?
I guess the randomness comes from HashMap, but since now you use serde_json::Value to do the check, I guess the order doesn't matter?
There was a problem hiding this comment.
Seems it still matters in below outtput. One approach is to change this function to make it determined:
#[cfg(test)]
// sort the keys.
| "referenced_data_file": null, | ||
| "content_offset": null, | ||
| "content_size_in_bytes": null | ||
| }); |
There was a problem hiding this comment.
Also I found a tricky corner case: DataFile's serializer won't preserve the order of DataFile's array like column_sizes. Right now I just make the array sizes to be one to work around that, maybe there is a better solution?
I guess the randomness comes from HashMap, but since now you use serde_json::Value to do the check, I guess the order doesn't matter?
| "referenced_data_file": null, | ||
| "content_offset": null, | ||
| "content_size_in_bytes": null | ||
| }); |
There was a problem hiding this comment.
Seems it still matters in below outtput. One approach is to change this function to make it determined:
#[cfg(test)]
// sort the keys.
Which issue does this PR close?
What changes are included in this PR?
DataFileSerde::try_fromtake inFormatVersiondirectlyAre these changes tested?
Added uts