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 2022/02/04 13:37:50 UTC

[GitHub] [arrow-rs] zeevm opened a new pull request #1271: Make rle decoder public

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


   Closes #1270 
   
   # Are there any user-facing changes?
   
   'rle' decoder is exposed outside the crate.
   
   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] zeevm commented on pull request #1271: Make rle decoder public

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


   @tustvold  @alamb @sunchao I'm not too versed in github workflow,  is there anything else I should do to complete the PR?


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] sunchao merged pull request #1271: Make rle decoder public

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


   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] zeevm closed pull request #1271: Make rle decoder public

Posted by GitBox <gi...@apache.org>.
zeevm closed pull request #1271:
URL: https://github.com/apache/arrow-rs/pull/1271


   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] sunchao commented on pull request #1271: Make rle decoder public

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


   LGTM, merging to master.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] zeevm commented on pull request #1271: Make rle decoder public

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


   @alamb @sunchao I've changed it to be "experimental" as you suggested.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] zeevm commented on a change in pull request #1271: Make rle decoder public

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



##########
File path: parquet/src/encodings/mod.rs
##########
@@ -18,4 +18,4 @@
 pub mod decoding;
 pub mod encoding;
 pub mod levels;
-pub(crate) mod rle;
+pub mod rle;

Review comment:
       @tustvold I'd like to use 'decoding' as well, it makes it really easy to decode RLE encoded pages I get from a column page reader.




-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] codecov-commenter commented on pull request #1271: Make rle decoder public

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


   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1271?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 [#1271](https://codecov.io/gh/apache/arrow-rs/pull/1271?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9c1bb05) into [master](https://codecov.io/gh/apache/arrow-rs/commit/fa728736bffb44af2d2e7fd3a890678f3e807cfd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa72873) will **decrease** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-rs/pull/1271/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/1271?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    #1271      +/-   ##
   ==========================================
   - Coverage   83.02%   83.02%   -0.01%     
   ==========================================
     Files         180      180              
     Lines       52269    52269              
   ==========================================
   - Hits        43398    43396       -2     
   - Misses       8871     8873       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1271?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/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/1271/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.51% <0.00%> (-0.26%)` | :arrow_down: |
   | [parquet\_derive/src/parquet\_field.rs](https://codecov.io/gh/apache/arrow-rs/pull/1271/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-cGFycXVldF9kZXJpdmUvc3JjL3BhcnF1ZXRfZmllbGQucnM=) | `66.21% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1271?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/1271?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 [fa72873...9c1bb05](https://codecov.io/gh/apache/arrow-rs/pull/1271?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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] tustvold commented on a change in pull request #1271: Make rle decoder public

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



##########
File path: parquet/src/encodings/mod.rs
##########
@@ -18,4 +18,4 @@
 pub mod decoding;
 pub mod encoding;
 pub mod levels;
-pub(crate) mod rle;
+pub mod rle;

Review comment:
       Makes sense 👍




-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] zeevm commented on pull request #1271: Make rle decoder public

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


   What's the process for completing the PR?


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] tustvold commented on a change in pull request #1271: Make rle decoder public

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



##########
File path: parquet/src/encodings/mod.rs
##########
@@ -18,4 +18,4 @@
 pub mod decoding;
 pub mod encoding;
 pub mod levels;
-pub(crate) mod rle;
+pub mod rle;

Review comment:
       Perhaps if you're only interested in `rle` we could make the above modules `pub(crate)`?




-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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