alamb commented on code in PR #8543:
URL: https://github.com/apache/arrow-rs/pull/8543#discussion_r2406680916
##########
arrow-array/src/builder/boolean_builder.rs:
##########
@@ -234,9 +234,12 @@ impl ArrayBuilder for BooleanBuilder {
impl Extend<Option<bool>> for BooleanBuilder {
#[inline]
fn extend<T: IntoIterator<Item = Option<bool>>>(&mut self, iter: T) {
- for v in iter {
- self.append_option(v)
- }
+ let buffered = iter.into_iter().collect::<Vec<_>>();
+ let array = unsafe {
+ // SAFETY: buffered.into_iter() is a trusted length iterator
Review Comment:
I think technically `buffered` is a `Vec` which is a trusted length iterator
```suggestion
// SAFETY: Vec is a trusted length iterator
```
##########
arrow-array/src/array/boolean_array.rs:
##########
@@ -436,11 +436,84 @@ impl<'a> BooleanArray {
}
}
-impl<Ptr: std::borrow::Borrow<Option<bool>>> FromIterator<Ptr> for
BooleanArray {
+/// An optional boolean value
+///
+/// This struct is used as an adapter when creating `BooleanArray` from an
iterator.
+/// `FromIterator` for `BooleanArray` takes an iterator where the elements can
be `into`
+/// this struct. So once implementing `From` or `Into` trait for a type, an
iterator of
+/// the type can be collected to `BooleanArray`.
+///
+/// See also [NativeAdapter](crate::array::NativeAdapter).
+#[derive(Debug)]
+struct BooleanAdapter {
+ /// Corresponding Rust native type if available
+ pub native: Option<bool>,
+}
+
+impl From<bool> for BooleanAdapter {
+ fn from(value: bool) -> Self {
+ BooleanAdapter {
+ native: Some(value),
+ }
+ }
+}
+
+impl From<&bool> for BooleanAdapter {
+ fn from(value: &bool) -> Self {
+ BooleanAdapter {
+ native: Some(*value),
+ }
+ }
+}
+
+impl From<Option<bool>> for BooleanAdapter {
+ fn from(value: Option<bool>) -> Self {
+ BooleanAdapter { native: value }
+ }
+}
+
+impl From<&Option<bool>> for BooleanAdapter {
+ fn from(value: &Option<bool>) -> Self {
+ BooleanAdapter { native: *value }
+ }
+}
+
+impl<Ptr: Into<BooleanAdapter>> FromIterator<Ptr> for BooleanArray {
fn from_iter<I: IntoIterator<Item = Ptr>>(iter: I) -> Self {
let iter = iter.into_iter();
- let (_, data_len) = iter.size_hint();
- let data_len = data_len.expect("Iterator must be sized"); // panic if
no upper bound.
+ let capacity = match iter.size_hint() {
+ (lower, Some(upper)) if lower == upper => lower,
+ _ => 0,
+ };
+ let mut builder = BooleanBuilder::with_capacity(capacity);
+ builder.extend(iter.map(|item| item.into().native));
+ builder.finish()
+ }
+}
+
+impl BooleanArray {
+ /// Creates a [`BooleanArray`] from an iterator of trusted length.
+ ///
+ /// # Safety
Review Comment:
this is consistent with `PrimitiveArray::from_trusted_len_iter`:
https://github.com/apache/arrow-rs/blob/9ba0eebeebeef5e0e661926f631373587523fdcb/arrow-array/src/array/primitive_array.rs#L1460-L1463
👍
##########
arrow-array/src/array/boolean_array.rs:
##########
@@ -436,11 +436,84 @@ impl<'a> BooleanArray {
}
}
-impl<Ptr: std::borrow::Borrow<Option<bool>>> FromIterator<Ptr> for
BooleanArray {
+/// An optional boolean value
+///
+/// This struct is used as an adapter when creating `BooleanArray` from an
iterator.
+/// `FromIterator` for `BooleanArray` takes an iterator where the elements can
be `into`
+/// this struct. So once implementing `From` or `Into` trait for a type, an
iterator of
+/// the type can be collected to `BooleanArray`.
+///
+/// See also [NativeAdapter](crate::array::NativeAdapter).
Review Comment:
thank you for the reference to `NativeAdapter` -- I was wondering why this
seemingly new pattern
Given it follows the existing pattern, I say 👍
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
