-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 from_slice trait to ease arrow2 migration #1588
Conversation
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.
Great idea @jimexist -- thank you!
The code looks good to me
let array_data = ArrayData::builder(T::DATA_TYPE) | ||
.len(slice.len()) | ||
.add_buffer(Buffer::from_slice_ref(&slice)); | ||
let array_data = unsafe { array_data.build_unchecked() }; |
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 had to double check myself, but I think this is good sound (This is not valid for BooleanType
) but BooleanType
doesn't implement ArrowPrimitiveType
👍
https://docs.rs/arrow/7.0.0/arrow/datatypes/trait.ArrowPrimitiveType.html
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.
actually this was copied from the From<Vec
part in ArrowPrimitiveType
similar changes can be done for booleans and string arrays but it can happen later
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.
@alamb support for boolean, str, and bin array types are added in:
Thank you @jimexist |
Which issue does this PR close?
Closes #.
Rationale for this change
Per #1556 - this trait is added to ease migration to arrow2:
What changes are included in this PR?
Are there any user-facing changes?