Replace serde_ndim feature with per-call-site Nested wrapper (#32) - #33
Merged
Conversation
serde_ndim switched the write format for arrays globally via a Cargo feature, which silently affects every other ninterp consumer in the same binary since features are additive. It was also broken for non-self-describing formats (bincode, postcard): deserialize_any is unconditional, and fixed-size grids serialize as tuples, which desyncs the byte stream on read. Replace it with a Nested wrapper / serialize_nested helper (exposed via prelude) that opts into the nested-array format at the point of serialization. The ndarray format stays the derive default: fastest, and the only format that works with binary serializers. Reading already accepted either format and continues to. Gate the tolerant reader on is_human_readable() so non-self-describing formats fall back to the ndarray format instead of failing, and fix the fixed-size grid tuple/seq desync. tests/serde_formats.rs round-trips every serializable type through both formats (catches the hand-written SerializeNested impls drifting from derived Deserialize) plus a bincode round-trip regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #32.
serde_ndimswitched the write format for arrays globally via a Cargo feature. Since Cargofeatures are additive and unify across the dependency graph, enabling it anywhere in a binary
silently flipped the wire format for every other
ninterpconsumer in that binary — no way toopt in per call site. It was also broken for non-self-describing formats (bincode, postcard) in
both feature configurations:
deserialize_anywas called unconditionally, which those formatsdon't support at all, and fixed-size grids (
[ArrayBase<D, Ix1>; N]) serialize as a tuple, whichthose formats encode without a length prefix, so reading it back as a seq desynchronized the byte
stream.
What changed
The
ndarrayformat stays the derive default — fastest to parse, and the only format that workswith binary serializers. The nested format is now opt-in at the point of serialization:
Both are exposed via
prelude, alongside everything else the crate re-exports for common use.Reading is untouched and still accepts either format regardless of which one wrote it — this is
purely about what gets written. The tolerant reader (and the fixed-grid tuple/seq handling) is
now gated on
Deserializer::is_human_readable(), so non-self-describing formats fall back to thendarrayformat instead of failing outright.Testing
tests/serde_formats.rsround-trips every serializable type through both formats — theSerializeNestedimpls are hand-written (serde has no channel to thread a format choice down avalue, so each level re-wraps its children), so this is what catches them drifting out of sync
with the derived
Deserialize. Also covers: everyInterpolatorEnumvariant specifically (sinceuntagged enums fail confusingly when field names shift),
Nestedreaching throughVec/Optioncontainers,
serialize_withon a field of an external struct, and a bincode round-trip as theregression test for the two format-agnostic-reading bugs above.
cargo test --features serde/--no-default-features,cargo clippy --all-features --all-targets,cargo fmt --check, andcargo doc --all-featuresare all clean. All fourexamples run; benches compile.
Breaking
serde_ndimno longer exists as a feature. Migrate to wrapping values inNested(orserialize_with = "serialize_nested"on a field) at the call site that wants the nested format.Data written by prior versions still reads fine either way, since the reader was already
format-agnostic.