Conversation
|
For your review @alamb |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
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
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. |
ByteBaker
force-pushed
the
master
branch
2 times, most recently
from
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. |
| /// ("b", b), | ||
| /// ]); | ||
| /// ``` | ||
| /// Another way to quickly create a [`RecordBatch`] is to use the [`record_batch!`] macro, |
|
Thanks @ByteBaker and @timsaucer |
alamb
added
the
enhancement
Removes DataFusion's custom `record_batch!` and `create_array!` macro implementations in favor of the upstream versions from arrow-rs added in apache/arrow-rs#6588. Changes: - Replace custom macro definitions with re-exports from arrow::array - Update syntax from vec![...] to array literal [...] across 67 usages - Add arrow_schema aliases in test modules for macro compatibility - Replace macro usage with manual RecordBatch construction where variables are used (macros only support literals) Closes apache#13037
Removes DataFusion's custom `record_batch!` and `create_array!` macro implementations in favor of the upstream versions from arrow-rs added in apache/arrow-rs#6588. Changes: - Replace custom macro definitions with re-exports from arrow::array - Update syntax from vec![...] to array literal [...] across 67 usages - Add arrow_schema aliases in test modules for macro compatibility - Replace macro usage with manual RecordBatch construction where variables are used (macros only support literals) Closes apache#13037
Removes DataFusion's custom `record_batch!` and `create_array!` macro implementations in favor of the upstream versions from arrow-rs added in apache/arrow-rs#6588. Changes: - Replace custom macro definitions with re-exports from arrow::array - Update syntax from vec![...] to array literal [...] across 67 usages - Add arrow_schema aliases in test modules for macro compatibility - Add record_batch_vec! legacy macro for cases where upstream macro cannot be used (variables, vec repetition syntax) - Use record_batch_vec! in examples and tests that pass variables - Add datafusion-common dependency to ffi_example_table_provider The record_batch_vec! macro is a temporary solution until arrow-rs supports variables and repetition syntax. See upstream issue: apache/arrow-rs#6553 Closes apache#13037
3 participants
closes: #6553