-
Notifications
You must be signed in to change notification settings - Fork 819
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
feat: record_batch!
macro
#6588
Conversation
For your review @alamb |
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.
Thank you @ByteBaker -- this looks great to me.
I think it might also be nice to put a link to this macro on other docs
https://docs.rs/arrow/latest/arrow/record_batch/struct.RecordBatch.html#method.try_from_iter
And also to add an example to the the RecordBatch docs themselves https://docs.rs/arrow/latest/arrow/record_batch/struct.RecordBatch.html
cc @timsaucer
Thank you for doing this! |
@alamb you're right. I was just lazy and did the bare-minimum. Lemme see how I can further extend this to cover more types and add necessary docs wherever applicable. I'll be back soon. |
Marking as draft as the CI is not passing and there seems to be planned work. CI might pass if we merge up from main |
@alamb sorry but I've been somewhat occupied the past several weeks. There's some work I've done but I don't think it's quite ready yet. |
e05d26d
to
e4310ab
Compare
@alamb @timsaucer I've synced the repo and pushed the work for now. Please take a look. I believe this covers most, if not all the non-nested types. I've also added the necessary docs, clearly mentioning the limitations of this macro. |
Oh nice. FWIW, I don’t think we need to cover nested types. This is really just a convenience macro and primarily used for unit tests and examples, so it’s perfectly reasonable in my opinion that the scope is limited. For anything more the user can do what they’ve been doing until now. I will try to review tomorrow or Monday unless someone gets to it first. |
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.
Love it
@@ -411,6 +547,19 @@ impl RecordBatch { | |||
/// ("b", b), | |||
/// ]); | |||
/// ``` | |||
/// Another way to quickly create a [`RecordBatch`] is to use the [`record_batch!`] macro, |
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.
💯
Thanks @ByteBaker and @timsaucer |
closes: #6553