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/05/22 08:40:28 UTC

[GitHub] [arrow-rs] novemberkilo opened a new pull request #338: Doctests for BooleanArray.

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


   # Which issue does this PR close?
   Closes #301 .
   
    # What changes are included in this PR?
   Doctests for `BooleanArray`
   
   # Are there any user-facing changes?
   No.


-- 
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 #338: Doctests for BooleanArray.

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


   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/338?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 [#338](https://codecov.io/gh/apache/arrow-rs/pull/338?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8325ecd) into [master](https://codecov.io/gh/apache/arrow-rs/commit/71c21595a0856e4f87838c8c3bb5f2b04fb88024?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71c2159) will **increase** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-rs/pull/338/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/338?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     #338   +/-   ##
   =======================================
     Coverage   82.53%   82.53%           
   =======================================
     Files         162      162           
     Lines       44043    44043           
   =======================================
   + Hits        36350    36351    +1     
   + Misses       7693     7692    -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/338?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/array\_boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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-YXJyb3cvc3JjL2FycmF5L2FycmF5X2Jvb2xlYW4ucnM=) | `90.90% <ø> (ø)` | |
   | [arrow/src/array/transform/boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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/338?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/338?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 [71c2159...8325ecd](https://codecov.io/gh/apache/arrow-rs/pull/338?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] alamb commented on pull request #338: Doctests for BooleanArray.

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


   the MIRI failure is unrelated to this PR -- it is happening on master too -- see https://github.com/apache/arrow-rs/issues/345.
   
   Thanks again @novemberkilo !


-- 
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 merged pull request #338: Doctests for BooleanArray.

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


   


-- 
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 #338: Doctests for BooleanArray.

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



##########
File path: arrow/src/array/array_boolean.rs
##########
@@ -27,6 +27,28 @@ use crate::buffer::{Buffer, MutableBuffer};
 use crate::util::bit_util;
 
 /// Array of bools
+///
+/// # Example
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     let arr = BooleanArray::from(vec![Some(false), Some(true), None, Some(true)]);
+///     assert_eq!(4, arr.len());
+///     assert_eq!(0, arr.offset());

Review comment:
       ```suggestion
   ```




-- 
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] novemberkilo commented on pull request #338: Doctests for BooleanArray.

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


   Looks like the build failure is due to a configuration issue (`rustup` not found on the windows instance)?


-- 
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 #338: Doctests for BooleanArray.

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



##########
File path: arrow/src/array/array_boolean.rs
##########
@@ -27,6 +27,86 @@ use crate::buffer::{Buffer, MutableBuffer};
 use crate::util::bit_util;
 
 /// Array of bools
+///
+/// # Example **using a builder**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     let mut builder = BooleanArray::builder(3);
+///     builder.append_value(true).unwrap();
+///     builder.append_null().unwrap();
+///     builder.append_value(false).unwrap();
+///     let arr = builder.finish();
+///     assert_eq!(3, arr.len());
+///     assert_eq!(1, arr.null_count());
+/// ```
+///
+/// # Example **from a vector**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     use arrow::buffer::Buffer;
+///     let buf = Buffer::from([10_u8]);
+///     let arr = BooleanArray::from(vec![false, true, false, true]);

Review comment:
       I think a single example from `Option<bool>` would be good enough. 

##########
File path: arrow/src/array/array_boolean.rs
##########
@@ -27,6 +27,86 @@ use crate::buffer::{Buffer, MutableBuffer};
 use crate::util::bit_util;
 
 /// Array of bools
+///
+/// # Example **using a builder**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     let mut builder = BooleanArray::builder(3);
+///     builder.append_value(true).unwrap();
+///     builder.append_null().unwrap();
+///     builder.append_value(false).unwrap();
+///     let arr = builder.finish();
+///     assert_eq!(3, arr.len());
+///     assert_eq!(1, arr.null_count());
+/// ```
+///
+/// # Example **from a vector**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     use arrow::buffer::Buffer;
+///     let buf = Buffer::from([10_u8]);
+///     let arr = BooleanArray::from(vec![false, true, false, true]);
+///     assert_eq!(&buf, arr.values());
+///     assert_eq!(4, arr.len());
+///     assert_eq!(0, arr.offset());
+///     assert_eq!(0, arr.null_count());
+///     for i in 0..4 {
+///        assert!(!arr.is_null(i));
+///        assert!(arr.is_valid(i));
+///        assert_eq!(i == 1 || i == 3, arr.value(i), "failed at {}", i)
+///     }
+/// ```
+///
+/// # Example **from a vector of Option values**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     use arrow::buffer::Buffer;
+///     let buf = Buffer::from([10_u8]);
+///     let arr = BooleanArray::from(vec![Some(false), Some(true), None, Some(true)]);
+///     assert_eq!(&buf, arr.values());
+///     assert_eq!(4, arr.len());
+///     assert_eq!(0, arr.offset());
+///     assert_eq!(1, arr.null_count());
+///     for i in 0..4 {
+///         if i == 2 {
+///             assert!(arr.is_null(i));
+///             assert!(!arr.is_valid(i));
+///         } else {
+///             assert!(!arr.is_null(i));
+///             assert!(arr.is_valid(i));
+///             assert_eq!(i == 1 || i == 3, arr.value(i), "failed at {}", i)
+///         }
+///     }
+/// ```
+///
+/// # Example **using an array builder**

Review comment:
       I think the array data version is a more advanced feature and we should probably not put that example on `BooleanArray` (perhaps it would go better on `ArrayDataBuilder`?)

##########
File path: arrow/src/array/array_boolean.rs
##########
@@ -27,6 +27,86 @@ use crate::buffer::{Buffer, MutableBuffer};
 use crate::util::bit_util;
 
 /// Array of bools
+///
+/// # Example **using a builder**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     let mut builder = BooleanArray::builder(3);
+///     builder.append_value(true).unwrap();
+///     builder.append_null().unwrap();
+///     builder.append_value(false).unwrap();
+///     let arr = builder.finish();
+///     assert_eq!(3, arr.len());
+///     assert_eq!(1, arr.null_count());
+/// ```
+///
+/// # Example **from a vector**
+///
+/// ```
+///     use arrow::array::{Array, BooleanArray};
+///     use arrow::buffer::Buffer;
+///     let buf = Buffer::from([10_u8]);
+///     let arr = BooleanArray::from(vec![false, true, false, true]);
+///     assert_eq!(&buf, arr.values());

Review comment:
       I think the use of `Buffer` here is confusing as Buffer shouldn't be needed for most usecases. 
   
   What about just illustrating the resulting array with some calls like
   
   ```
   assert_eq!(arr.is_valid(0))
   assert_eq!(true, arr.value(0))
   ```
   etc




-- 
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] novemberkilo commented on pull request #338: Doctests for BooleanArray.

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


   Thanks @alamb - I have put up changes to address your comments, squashed and pushed so that I have a single commit here again. 


-- 
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 #338: Doctests for BooleanArray.

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


   > Looks like the build failure is due to a configuration issue (rustup not found on the windows instance)?
   
   I agree -- I will restart the tests 


-- 
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 edited a comment on pull request #338: Doctests for BooleanArray.

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #338:
URL: https://github.com/apache/arrow-rs/pull/338#issuecomment-846377397


   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/338?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 [#338](https://codecov.io/gh/apache/arrow-rs/pull/338?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c1abbe9) into [master](https://codecov.io/gh/apache/arrow-rs/commit/b2de5446cc1e45a0559fb39039d0545df1ac0d26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2de544) will **increase** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-rs/pull/338/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/338?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     #338      +/-   ##
   ==========================================
   + Coverage   82.54%   82.56%   +0.01%     
   ==========================================
     Files         162      162              
     Lines       44047    44063      +16     
   ==========================================
   + Hits        36359    36379      +20     
   + Misses       7688     7684       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/338?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/array\_boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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-YXJyb3cvc3JjL2FycmF5L2FycmF5X2Jvb2xlYW4ucnM=) | `90.90% <ø> (ø)` | |
   | [arrow/src/array/transform/boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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) | `76.92% <0.00%> (-7.70%)` | :arrow_down: |
   | [arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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%> (+0.04%)` | :arrow_up: |
   | [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `95.04% <0.00%> (+0.19%)` | :arrow_up: |
   | [arrow/src/array/equal/list.rs](https://codecov.io/gh/apache/arrow-rs/pull/338/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL2xpc3QucnM=) | `89.55% <0.00%> (+5.97%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/338?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/338?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 [b2de544...c1abbe9](https://codecov.io/gh/apache/arrow-rs/pull/338?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