Repository: jorgecarleitao/arrow2
Pull request #297 (2021-08-18 08:31:36)

Improved `MutableBooleanArray`
* Added `PartialEq`
* Added `.iter`, `values_iter` and `IntoIterator`
* Added `from` slice of `Option<bool>`
* Improved tests to its API.


codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#297](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (7a6f1e7) into [main](https://codecov.io/gh/jorgecarleitao/arrow2/commit/fa3b003759d5a98200a98ed93facb8751f3aa6a0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (fa3b003) will **increase** coverage by `0.04%`.
> The diff coverage is `94.52%`.

[![Impacted file tree graph](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/graphs/tree.svg?width=650&height=150&src=pr&token=AgyTF60R3D&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)

```diff
@@            Coverage Diff             @@
##             main     #297      +/-   ##
==========================================
+ Coverage   77.51%   77.56%   +0.04%     
==========================================
  Files         315      315              
  Lines       20967    20995      +28     
==========================================
+ Hits        16253    16284      +31     
+ Misses       4714     4711       -3     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/bitmap/mutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC9tdXRhYmxlLnJz) | `88.31% <66.66%> (-0.82%)` | :arrow_down: |
| [src/array/boolean/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2Jvb2xlYW4vaXRlcmF0b3IucnM=) | `57.14% <77.77%> (-9.53%)` | :arrow_down: |
| [src/array/binary/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2JpbmFyeS9pdGVyYXRvci5ycw==) | `87.50% <100.00%> (+0.83%)` | :arrow_up: |
| [src/array/boolean/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2Jvb2xlYW4vbW9kLnJz) | `81.81% <100.00%> (ø)` | |
| [src/array/boolean/mutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2Jvb2xlYW4vbXV0YWJsZS5ycw==) | `66.18% <100.00%> (+7.66%)` | :arrow_up: |
| [src/array/dictionary/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2RpY3Rpb25hcnkvaXRlcmF0b3IucnM=) | `50.00% <100.00%> (+2.38%)` | :arrow_up: |
| [src/array/fixed\_size\_binary/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2ZpeGVkX3NpemVfYmluYXJ5L2l0ZXJhdG9yLnJz) | `64.28% <100.00%> (+2.74%)` | :arrow_up: |
| [src/array/fixed\_size\_list/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2ZpeGVkX3NpemVfbGlzdC9pdGVyYXRvci5ycw==) | `55.55% <100.00%> (+5.55%)` | :arrow_up: |
| [src/array/list/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2xpc3QvaXRlcmF0b3IucnM=) | `62.50% <100.00%> (+1.63%)` | :arrow_up: |
| [src/array/primitive/iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L3ByaW1pdGl2ZS9pdGVyYXRvci5ycw==) | `60.00% <100.00%> (+10.00%)` | :arrow_up: |
| ... and [8 more](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao).
> **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=Jorge+Leitao)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [fa3b003...7a6f1e7](https://codecov.io/gh/jorgecarleitao/arrow2/pull/297?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). 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=Jorge+Leitao).


Patch:
diff --git a/src/array/boolean/iterator.rs b/src/array/boolean/iterator.rs
index 9b2d3821da..d5e0b0c773 100644
--- a/src/array/boolean/iterator.rs
+++ b/src/array/boolean/iterator.rs
@@ -1,6 +1,7 @@
 use crate::bitmap::utils::{zip_validity, BitmapIter, ZipValidity};
 
-use super::BooleanArray;
+use super::super::MutableArray;
+use super::{BooleanArray, MutableBooleanArray};
 
 impl<'a> IntoIterator for &'a BooleanArray {
     type Item = Option<bool>;
@@ -28,3 +29,30 @@ impl<'a> BooleanArray {
         self.values().iter()
     }
 }
+
+impl<'a> IntoIterator for &'a MutableBooleanArray {
+    type Item = Option<bool>;
+    type IntoIter = ZipValidity<'a, bool, BitmapIter<'a>>;
+
+    #[inline]
+    fn into_iter(self) -> Self::IntoIter {
+        self.iter()
+    }
+}
+
+impl<'a> MutableBooleanArray {
+    /// Returns an iterator over `Option<bool>`
+    #[inline]
+    pub fn iter(&'a self) -> ZipValidity<'a, bool, BitmapIter<'a>> {
+        zip_validity(
+            self.values().iter(),
+            self.validity().as_ref().map(|x| x.iter()),
+        )
+    }
+
+    /// Returns an iterator of `bool`
+    #[inline]
+    pub fn values_iter(&'a self) -> BitmapIter<'a> {
+        self.values().iter()
+    }
+}
diff --git a/src/array/boolean/mod.rs b/src/array/boolean/mod.rs
index 212bc01163..43c310b300 100644
--- a/src/array/boolean/mod.rs
+++ b/src/array/boolean/mod.rs
@@ -125,8 +125,7 @@ impl std::fmt::Display for BooleanArray {
 
 impl<P: AsRef<[Option<bool>]>> From<P> for BooleanArray {
     /// Creates a new [`BooleanArray`] out of a slice of Optional `bool`.
-    #[inline]
     fn from(slice: P) -> Self {
-        Self::from_trusted_len_iter(slice.as_ref().iter().map(|x| x.as_ref()))
+        MutableBooleanArray::from(slice).into()
     }
 }
diff --git a/src/array/boolean/mutable.rs b/src/array/boolean/mutable.rs
index 44231cc89a..d5099a4820 100644
--- a/src/array/boolean/mutable.rs
+++ b/src/array/boolean/mutable.rs
@@ -25,6 +25,13 @@ impl From<MutableBooleanArray> for BooleanArray {
     }
 }
 
+impl<P: AsRef<[Option<bool>]>> From<P> for MutableBooleanArray {
+    /// Creates a new [`MutableBooleanArray`] out of a slice of Optional `bool`.
+    fn from(slice: P) -> Self {
+        Self::from_trusted_len_iter(slice.as_ref().iter().map(|x| x.as_ref()))
+    }
+}
+
 impl Default for MutableBooleanArray {
     fn default() -> Self {
         Self::new()
@@ -74,11 +81,10 @@ impl MutableBooleanArray {
     }
 
     fn init_validity(&mut self) {
-        self.validity = Some(MutableBitmap::from_trusted_len_iter(
-            std::iter::repeat(true)
-                .take(self.len() - 1)
-                .chain(std::iter::once(false)),
-        ))
+        let mut validity = MutableBitmap::new();
+        validity.extend_constant(self.len(), true);
+        validity.set(self.len() - 1, false);
+        self.validity = Some(validity)
     }
 
     /// Converts itself into an [`Array`].
@@ -360,3 +366,9 @@ impl TryExtend<Option<bool>> for MutableBooleanArray {
         Ok(())
     }
 }
+
+impl PartialEq for MutableBooleanArray {
+    fn eq(&self, other: &Self) -> bool {
+        self.iter().eq(other.iter())
+    }
+}
diff --git a/src/bitmap/mutable.rs b/src/bitmap/mutable.rs
index e97934b8d1..eed57fcdfc 100644
--- a/src/bitmap/mutable.rs
+++ b/src/bitmap/mutable.rs
@@ -78,10 +78,8 @@ impl MutableBitmap {
         if self.length % 8 == 0 {
             self.buffer.push(0);
         }
-        if value {
-            let byte = self.buffer.as_mut_slice().last_mut().unwrap();
-            *byte = set(*byte, self.length % 8, true);
-        };
+        let byte = self.buffer.as_mut_slice().last_mut().unwrap();
+        *byte = set(*byte, self.length % 8, value);
         self.length += 1;
     }
 
@@ -217,11 +215,6 @@ impl MutableBitmap {
         assert!(length <= buffer.len() * 8);
         Self { buffer, length }
     }
-
-    /// Returns an iterator over the values of the [`MutableBitmap`].
-    fn iter(&self) -> BitmapIter {
-        BitmapIter::new(&self.buffer, 0, self.len())
-    }
 }
 
 impl From<MutableBitmap> for Bitmap {
@@ -546,3 +539,19 @@ impl Default for MutableBitmap {
         Self::new()
     }
 }
+
+impl<'a> IntoIterator for &'a MutableBitmap {
+    type Item = bool;
+    type IntoIter = BitmapIter<'a>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        BitmapIter::<'a>::new(&self.buffer, 0, self.length)
+    }
+}
+
+impl<'a> MutableBitmap {
+    /// constructs a new iterator over the values of [`MutableBitmap`].
+    pub fn iter(&'a self) -> BitmapIter<'a> {
+        BitmapIter::<'a>::new(&self.buffer, 0, self.length)
+    }
+}


Test patch:
diff --git a/tests/it/array/boolean/mutable.rs b/tests/it/array/boolean/mutable.rs
index 9408c1e89d..4ea81b15ff 100644
--- a/tests/it/array/boolean/mutable.rs
+++ b/tests/it/array/boolean/mutable.rs
@@ -1,30 +1,40 @@
-use arrow2::{
-    array::{MutableArray, MutableBooleanArray},
-    bitmap::MutableBitmap,
-};
+use arrow2::array::{MutableArray, MutableBooleanArray};
+
+#[test]
+fn set() {
+    let mut a = MutableBooleanArray::from(&[Some(false), Some(true), Some(false)]);
+
+    a.set(1, None);
+    a.set(0, Some(true));
+    assert_eq!(
+        a,
+        MutableBooleanArray::from([Some(true), None, Some(false)])
+    )
+}
 
 #[test]
 fn push() {
     let mut a = MutableBooleanArray::new();
     a.push(Some(true));
+    a.push(Some(false));
     a.push(None);
-    assert_eq!(a.len(), 2);
-    assert!(a.is_valid(0));
-    assert!(!a.is_valid(1));
-
-    assert_eq!(a.values(), &MutableBitmap::from([true, false]));
+    a.push_null();
+    assert_eq!(
+        a,
+        MutableBooleanArray::from([Some(true), Some(false), None, None])
+    );
 }
 
 #[test]
 fn from_trusted_len_iter() {
     let iter = std::iter::repeat(true).take(2).map(Some);
     let a = MutableBooleanArray::from_trusted_len_iter(iter);
-    assert_eq!(a.len(), 2);
+    assert_eq!(a, MutableBooleanArray::from([Some(true), Some(true)]));
 }
 
 #[test]
 fn from_iter() {
     let iter = std::iter::repeat(true).take(2).map(Some);
     let a: MutableBooleanArray = iter.collect();
-    assert_eq!(a.len(), 2);
+    assert_eq!(a, MutableBooleanArray::from([Some(true), Some(true)]));
 }