-
Notifications
You must be signed in to change notification settings - Fork 373
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
New helper for sticking Serde-encodable data into arrow #2004
Changes from 4 commits
91faa17
e3fb5ae
7cdc4af
7ac1896
57cce2a
c62b73a
ee3fef1
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,126 @@ | ||
use arrow2::{ | ||
array::{BinaryArray, MutableBinaryArray, TryPush}, | ||
datatypes::DataType, | ||
}; | ||
use arrow2_convert::{deserialize::ArrowDeserialize, field::ArrowField, serialize::ArrowSerialize}; | ||
|
||
/// Helper for storing arbitrary serde-compatible types in an arrow2 [`BinaryArray`] field | ||
/// | ||
/// Use as: | ||
/// ``` | ||
/// use re_log_types::serde_field::SerdeField; | ||
/// use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize, field::ArrowField}; | ||
/// use arrow2::datatypes::{DataType, Field}; | ||
/// | ||
/// #[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize)] | ||
/// struct SomeStruct { | ||
/// foo: String, | ||
/// bar: u32, | ||
/// } | ||
/// | ||
/// #[derive(Clone, Debug, PartialEq, ArrowField, ArrowSerialize, ArrowDeserialize)] | ||
/// struct SomeArrowStruct { | ||
/// #[arrow_field(type = "SerdeField<SomeStruct>")] | ||
/// field: SomeStruct, | ||
/// } | ||
/// | ||
/// assert_eq!( | ||
/// SomeArrowStruct::data_type(), | ||
/// DataType::Struct(vec![ | ||
/// Field::new("field", DataType::Binary, false), | ||
/// ]) | ||
/// ); | ||
/// ``` | ||
pub struct SerdeField<T>(std::marker::PhantomData<T>); | ||
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 wonder how we should handle the fact that we use MsgPack as the "codec". There are alternatives, like We could, for instance, add an 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 think the "right" thing to do in theory is to add this to the arrow meta-data for the field. We've been wanting to use that for some other stuff as well so this might be a good opportunity to investigate the plumbing. |
||
|
||
impl<T> ArrowField for SerdeField<T> | ||
where | ||
T: serde::ser::Serialize + serde::de::DeserializeOwned, | ||
{ | ||
type Type = T; | ||
|
||
#[inline] | ||
fn data_type() -> DataType { | ||
arrow2::datatypes::DataType::Binary | ||
} | ||
} | ||
|
||
impl<T> ArrowSerialize for SerdeField<T> | ||
where | ||
T: serde::ser::Serialize + serde::de::DeserializeOwned, | ||
{ | ||
type MutableArrayType = MutableBinaryArray<i32>; | ||
#[inline] | ||
fn new_array() -> Self::MutableArrayType { | ||
MutableBinaryArray::new() | ||
} | ||
|
||
fn arrow_serialize( | ||
v: &<Self as ArrowField>::Type, | ||
array: &mut Self::MutableArrayType, | ||
) -> arrow2::error::Result<()> { | ||
let mut buf = Vec::new(); | ||
rmp_serde::encode::write_named(&mut buf, v).map_err(|_err| { | ||
// TODO(jleibs): Could not get re_error::format() to work here | ||
jleibs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
arrow2::error::Error::ExternalFormat("Could not encode as rmp".to_owned()) | ||
})?; | ||
array.try_push(Some(buf)) | ||
} | ||
} | ||
|
||
impl<T> ArrowDeserialize for SerdeField<T> | ||
where | ||
T: serde::ser::Serialize + serde::de::DeserializeOwned, | ||
{ | ||
type ArrayType = BinaryArray<i32>; | ||
|
||
#[inline] | ||
fn arrow_deserialize(v: <&Self::ArrayType as IntoIterator>::Item) -> Option<T> { | ||
v.and_then(|v| rmp_serde::from_slice::<T>(v).ok()) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::SerdeField; | ||
use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize}; | ||
|
||
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize)] | ||
struct SomeStruct { | ||
foo: String, | ||
bar: u32, | ||
} | ||
|
||
#[derive(Clone, Debug, PartialEq, ArrowField, ArrowSerialize, ArrowDeserialize)] | ||
struct SomeArrowStruct { | ||
#[arrow_field(type = "SerdeField<SomeStruct>")] | ||
field: SomeStruct, | ||
} | ||
|
||
impl From<SomeStruct> for SomeArrowStruct { | ||
fn from(s: SomeStruct) -> Self { | ||
Self { field: s } | ||
} | ||
} | ||
|
||
#[test] | ||
fn round_trip_serdefield() { | ||
use arrow2_convert::{deserialize::TryIntoCollection, serialize::TryIntoArrow}; | ||
|
||
let data: [SomeArrowStruct; 2] = [ | ||
SomeStruct { | ||
foo: "hello".into(), | ||
bar: 42, | ||
} | ||
.into(), | ||
SomeStruct { | ||
foo: "world".into(), | ||
bar: 1983, | ||
} | ||
.into(), | ||
]; | ||
let array: Box<dyn arrow2::array::Array> = data.try_into_arrow().unwrap(); | ||
let ret: Vec<SomeArrowStruct> = array.try_into_collection().unwrap(); | ||
assert_eq!(&data, ret.as_slice()); | ||
} | ||
} |
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.
The extra struct layer will add a lot of unnecessary noise all over; and since people are most likely going to copy-paste this as a starting point, I'd rather show them how to avoid that straight from the start:
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.
Not sure I fully agree. Demoing that this can be done on just a sub-field is also helpful. But onboard with making the docstring shorter.