You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/06/14 08:00:31 UTC

[GitHub] [arrow-rs] LaurentMazare opened a new pull request #451: Implement the Iterator trait for the json Reader.

LaurentMazare opened a new pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451


   # 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?
   
   - Implement the `Iterator` trait directly on `json::Reader`.
   - Add test reading some json file through a for loop using this iterator.
   
   # Are there any user-facing changes?
   
   None expected, this adds a `next` function as part of the trait, there is already a public `next` function defined on `json::Reader` and this doesn't seem to be an issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] codecov-commenter commented on pull request #451: Implement the Iterator trait for the json Reader.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451#issuecomment-860053304


   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#451](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9aec343) into [master](https://codecov.io/gh/apache/arrow-rs/commit/71e9d783daa32d0c30d4181161305b360e81b69f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e9d78) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-rs/pull/451/graphs/tree.svg?width=650&height=150&src=pr&token=pq9V9qWZ1N&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #451   +/-   ##
   =======================================
     Coverage   82.65%   82.66%           
   =======================================
     Files         164      164           
     Lines       45468    45481   +13     
   =======================================
   + Hits        37581    37596   +15     
   + Misses       7887     7885    -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [arrow/src/json/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2pzb24vcmVhZGVyLnJz) | `83.91% <100.00%> (+0.26%)` | :arrow_up: |
   | [arrow/src/array/array\_dictionary.rs](https://codecov.io/gh/apache/arrow-rs/pull/451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X2RpY3Rpb25hcnkucnM=) | `84.56% <0.00%> (-0.32%)` | :arrow_down: |
   | [arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow-rs/pull/451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9jYXN0LnJz) | `94.53% <0.00%> (ø)` | |
   | [arrow/src/array/transform/boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9ib29sZWFuLnJz) | `84.61% <0.00%> (+7.69%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [71e9d78...9aec343](https://codecov.io/gh/apache/arrow-rs/pull/451?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] LaurentMazare commented on a change in pull request #451: Implement the Iterator trait for the json Reader.

Posted by GitBox <gi...@apache.org>.
LaurentMazare commented on a change in pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451#discussion_r650391154



##########
File path: arrow/src/json/reader.rs
##########
@@ -1569,6 +1569,18 @@ impl ReaderBuilder {
     }
 }
 
+impl<R: Read> Iterator for Reader<R> {
+    type Item = Result<RecordBatch>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self.next() {

Review comment:
       Ah indeed that sounds more idiomatic, I just pushed this change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] alamb commented on pull request #451: Implement the Iterator trait for the json Reader.

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451#issuecomment-865881532


   This is planned for release in arrow-rs 4.4.0 FYI


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] alamb commented on a change in pull request #451: Implement the Iterator trait for the json Reader.

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451#discussion_r650389395



##########
File path: arrow/src/json/reader.rs
##########
@@ -1569,6 +1569,18 @@ impl ReaderBuilder {
     }
 }
 
+impl<R: Read> Iterator for Reader<R> {
+    type Item = Result<RecordBatch>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self.next() {

Review comment:
       I think you might be able to do `self.next().transpose()` here 

##########
File path: arrow/src/json/reader.rs
##########
@@ -1569,6 +1569,14 @@ impl ReaderBuilder {
     }
 }
 
+impl<R: Read> Iterator for Reader<R> {
+    type Item = Result<RecordBatch>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.next().transpose()

Review comment:
       ❤️ 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] nevi-me merged pull request #451: Implement the Iterator trait for the json Reader.

Posted by GitBox <gi...@apache.org>.
nevi-me merged pull request #451:
URL: https://github.com/apache/arrow-rs/pull/451


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org