-
Notifications
You must be signed in to change notification settings - Fork 810
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
Implement the Iterator trait for the json Reader. #451
Conversation
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 for the contribution @LaurentMazare ! This looks like a great improvement to me.
arrow/src/json/reader.rs
Outdated
type Item = Result<RecordBatch>; | ||
|
||
fn next(&mut self) -> Option<Self::Item> { | ||
match self.next() { |
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 think you might be able to do self.next().transpose()
here
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.
Ah indeed that sounds more idiomatic, I just pushed this change.
type Item = Result<RecordBatch>; | ||
|
||
fn next(&mut self) -> Option<Self::Item> { | ||
self.next().transpose() |
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.
❤️
Codecov Report
@@ Coverage Diff @@
## master #451 +/- ##
=======================================
Coverage 82.65% 82.66%
=======================================
Files 164 164
Lines 45468 45481 +13
=======================================
+ Hits 37581 37596 +15
+ Misses 7887 7885 -2
Continue to review full report at Codecov.
|
* Implement the Iterator trait for the json Reader. * Use transpose.
This is planned for release in arrow-rs 4.4.0 FYI |
Which issue does this PR close?
Closes #193 (JSON reader does not implement iterator).
Rationale for this change
Fairly straightforward change that makes it easy to iterate over
RecordBatch
when reading a json file.What changes are included in this PR?
Iterator
trait directly onjson::Reader
.Are there any user-facing changes?
None expected, this adds a
next
function as part of the trait, there is already a publicnext
function defined onjson::Reader
and this doesn't seem to be an issue.