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

[C++] Add Buffer::data_as #36417

Closed
bkietz opened this issue Jun 30, 2023 · 0 comments · Fixed by #36418
Closed

[C++] Add Buffer::data_as #36417

bkietz opened this issue Jun 30, 2023 · 0 comments · Fixed by #36418

Comments

@bkietz
Copy link
Member

bkietz commented Jun 30, 2023

Describe the enhancement requested

Buffer's data is usually cast to a non-byte type before use, which requires a lot of boilerplate with reinterpret_cast. If the buffer happens to live in an ArrayData we can use GetValues but bare buffers don't have access to this. Let's add an accessor which does the same thing for bare buffers: const T* Buffer::data_as<T>() const and T* Buffer::mutable_data_as<T>() will greatly simplify usage of buffers.

Component(s)

C++

@bkietz bkietz self-assigned this Jun 30, 2023
pitrou pushed a commit that referenced this issue Jul 12, 2023
### Rationale for this change

There's a lot of boilerplate in casting buffer bytes. A templated accessor will decrease that.

### What changes are included in this PR?

- `{Buffer, BufferBuilder, BufferSpan}::{data_as<T>, mutable_data_as<T>}`
- rewriting a few files' worth of reinterpret_casts to use these accessors to showcase usage
- `Buffer::FromVector`, a std::vector analog to Buffer::FromString
- some tangential cleanup of unnecessary copies in the Array classes

### Are these changes tested?

Not really. The rewritten files exercise the new accessors

### Are there any user-facing changes?

No, but I've noticed `Buffer::operator[]` again and that should probably be deprecated.

* Closes: #36417

Authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 13.0.0 milestone Jul 12, 2023
@raulcd raulcd modified the milestones: 13.0.0, 14.0.0 Jul 13, 2023
chelseajonesr pushed a commit to chelseajonesr/arrow that referenced this issue Jul 20, 2023
…pache#36418)

### Rationale for this change

There's a lot of boilerplate in casting buffer bytes. A templated accessor will decrease that.

### What changes are included in this PR?

- `{Buffer, BufferBuilder, BufferSpan}::{data_as<T>, mutable_data_as<T>}`
- rewriting a few files' worth of reinterpret_casts to use these accessors to showcase usage
- `Buffer::FromVector`, a std::vector analog to Buffer::FromString
- some tangential cleanup of unnecessary copies in the Array classes

### Are these changes tested?

Not really. The rewritten files exercise the new accessors

### Are there any user-facing changes?

No, but I've noticed `Buffer::operator[]` again and that should probably be deprecated.

* Closes: apache#36417

Authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
…pache#36418)

### Rationale for this change

There's a lot of boilerplate in casting buffer bytes. A templated accessor will decrease that.

### What changes are included in this PR?

- `{Buffer, BufferBuilder, BufferSpan}::{data_as<T>, mutable_data_as<T>}`
- rewriting a few files' worth of reinterpret_casts to use these accessors to showcase usage
- `Buffer::FromVector`, a std::vector analog to Buffer::FromString
- some tangential cleanup of unnecessary copies in the Array classes

### Are these changes tested?

Not really. The rewritten files exercise the new accessors

### Are there any user-facing changes?

No, but I've noticed `Buffer::operator[]` again and that should probably be deprecated.

* Closes: apache#36417

Authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants