Skip to content
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

Migrate ArrowBuffer wrapper to expose arrow-rs buffers rather than arrow2 #2978

Closed
jleibs opened this issue Aug 14, 2023 · 0 comments · Fixed by #8201
Closed

Migrate ArrowBuffer wrapper to expose arrow-rs buffers rather than arrow2 #2978

jleibs opened this issue Aug 14, 2023 · 0 comments · Fixed by #8201
Assignees
Labels

Comments

@jleibs
Copy link
Member

jleibs commented Aug 14, 2023

#2970 introduced a new ArrowBuffer wrapper to return native arrow buffers on the deserialization path for Tensor types.

These buffers currently use arrow2::Buffer. In order to facilitate the future migration to arrow-rs, these can be made to wrap arrow-rs::Buffer instead. The types have convenient converters: https://docs.rs/arrow2/latest/arrow2/buffer/struct.Buffer.html#impl-From%3CBuffer%3CT%3E%3E-for-Buffer but we need to investigate whether these converters are still zero-copy.

One benefit of this migration is arrow-rs buffers use half:f16 whereas arrow2 buffers use arrow2::types::f16

@jleibs jleibs added enhancement New feature or request 👀 needs triage This issue needs to be triaged by the Rerun team 🏹 arrow concerning arrow codegen/idl and removed enhancement New feature or request 👀 needs triage This issue needs to be triaged by the Rerun team labels Aug 14, 2023
@emilk emilk self-assigned this Nov 21, 2024
emilk added a commit that referenced this issue Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants