-
Notifications
You must be signed in to change notification settings - Fork 307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add optional support for borsh serialisation #1335
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
use crate::imp_prelude::*; | ||
use crate::IntoDimension; | ||
use alloc::vec::Vec; | ||
use borsh::{BorshDeserialize, BorshSerialize}; | ||
use core::ops::Deref; | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl<I> BorshSerialize for Dim<I> | ||
where | ||
I: BorshSerialize, | ||
{ | ||
fn serialize<W: borsh::io::Write>(&self, writer: &mut W) -> borsh::io::Result<()> { | ||
<I as BorshSerialize>::serialize(&self.ix(), writer) | ||
} | ||
} | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl<I> BorshDeserialize for Dim<I> | ||
where | ||
I: BorshDeserialize, | ||
{ | ||
fn deserialize_reader<R: borsh::io::Read>(reader: &mut R) -> borsh::io::Result<Self> { | ||
<I as BorshDeserialize>::deserialize_reader(reader).map(Dim::new) | ||
} | ||
} | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl BorshSerialize for IxDyn { | ||
fn serialize<W: borsh::io::Write>(&self, writer: &mut W) -> borsh::io::Result<()> { | ||
let elts = self.ix().deref(); | ||
// Output length of dimensions. | ||
<usize as BorshSerialize>::serialize(&elts.len(), writer)?; | ||
// Followed by actual data. | ||
for elt in elts { | ||
<Ix as BorshSerialize>::serialize(elt, writer)?; | ||
} | ||
Ok(()) | ||
} | ||
} | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl BorshDeserialize for IxDyn { | ||
fn deserialize_reader<R: borsh::io::Read>(reader: &mut R) -> borsh::io::Result<Self> { | ||
// Deserialize the length. | ||
let len = <usize as BorshDeserialize>::deserialize_reader(reader)?; | ||
// Deserialize the given number of elements. We assume the source is | ||
// trusted so we use a capacity hint... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why trusted? |
||
let mut elts = Vec::with_capacity(len); | ||
for _ix in 0..len { | ||
elts.push(<Ix as BorshDeserialize>::deserialize_reader(reader)?); | ||
} | ||
Ok(elts.into_dimension()) | ||
} | ||
} | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl<A, D, S> BorshSerialize for ArrayBase<S, D> | ||
where | ||
A: BorshSerialize, | ||
D: Dimension + BorshSerialize, | ||
S: Data<Elem = A>, | ||
{ | ||
fn serialize<W: borsh::io::Write>(&self, writer: &mut W) -> borsh::io::Result<()> { | ||
// Dimensions | ||
<D as BorshSerialize>::serialize(&self.raw_dim(), writer)?; | ||
// Followed by length of data | ||
let iter = self.iter(); | ||
<usize as BorshSerialize>::serialize(&iter.len(), writer)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why both length and dimensions? |
||
// Followed by data itself. | ||
for elt in iter { | ||
<A as BorshSerialize>::serialize(elt, writer)?; | ||
} | ||
Ok(()) | ||
} | ||
} | ||
|
||
/// **Requires crate feature `"borsh"`** | ||
impl<A, D, S> BorshDeserialize for ArrayBase<S, D> | ||
where | ||
A: BorshDeserialize, | ||
D: BorshDeserialize + Dimension, | ||
S: DataOwned<Elem = A>, | ||
{ | ||
fn deserialize_reader<R: borsh::io::Read>(reader: &mut R) -> borsh::io::Result<Self> { | ||
// Dimensions | ||
let dim = <D as BorshDeserialize>::deserialize_reader(reader)?; | ||
// Followed by length of data | ||
let len = <usize as BorshDeserialize>::deserialize_reader(reader)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you handle integer size here? What if it was serialized on a 64-bit usize platform but you deserialize on a 32-bit? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would look at indexmap-rs/indexmap#313 for inspiration - fixed size integers and check error cases There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. borsh always serialises usize as u64: https://docs.rs/borsh/latest/src/borsh/ser/mod.rs.html#128-132 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. borsh says that container sizes should be stored as u32. Unsure how it interacts with ndarray. For me, developing this PR is not my priority unfortunately. Help from others welcome. |
||
// Followed by data itself. | ||
let mut data = Vec::with_capacity(len); | ||
for _ix in 0..len { | ||
data.push(<A as BorshDeserialize>::deserialize_reader(reader)?); | ||
} | ||
ArrayBase::from_shape_vec(dim, data).map_err(|_shape_err| { | ||
borsh::io::Error::new( | ||
borsh::io::ErrorKind::InvalidData, | ||
"data and dimensions must match in size", | ||
) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be worried that we have to break compatibility here if we change how dimensions are stored?