-
Notifications
You must be signed in to change notification settings - Fork 794
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 better documentation, examples and builer-style API to ByteView
#6479
Conversation
buffer_index: 3, | ||
offset: 1, | ||
}; | ||
let view = ByteView::new(13, &v.as_bytes()[0..4]) |
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.
This is an example of the kind of improvement the new API allows (aka not having to know to do try_into/unwrap for byte slices
FYI @XiangpengHao |
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 like it
Co-authored-by: Raphael Taylor-Davies <[email protected]>
Thank you for the review @tustvold |
Co-authored-by: Xiangpeng Hao <[email protected]>
}; | ||
let v: Vec<u8> = vec![ | ||
// invalid UTF8 | ||
0xf0, 0x80, 0x80, 0x80, // more bytes to make it larger than 12 |
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.
This was actually an invalid ByteView
(the length is less than 12 but the prefix is only 4), so I updated the test
Thanks again @tustvold and @XiangpengHao |
Which issue does this PR close?
Closes #6478
Rationale for this change
Rationale for this change
While working with StringViewArray's downstream in DataFusion, I found it was not always clear that
ByteView
exists for creating/destructuring with theu128
s so I wanted to write some additional documentationI also found it quite awkward to write the examples (specifically setting the prefix) so I added some BuilderStyle APIs as well
This is what creating ByteViews looks like without the builder APIs :vomit:
What changes are included in this PR?
GenericByteViewArray
from docsByteView
ByteView
Are there any user-facing changes?
There are several new functions on
ByteView