tobixdev commented on code in PR #8564:
URL: https://github.com/apache/arrow-rs/pull/8564#discussion_r2411961953
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -1458,14 +1458,21 @@ impl<T: ArrowPrimitiveType, Ptr:
Into<NativeAdapter<T>>> FromIterator<Ptr> for P
impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
/// Creates a [`PrimitiveArray`] from an iterator of trusted length.
+ ///
/// # Safety
+ ///
/// The iterator must be
[`TrustedLen`](https://doc.rust-lang.org/std/iter/trait.TrustedLen.html).
- /// I.e. that `size_hint().1` correctly reports its length.
+ /// I.e. that `size_hint().1` correctly reports its length. Note that this
is a stronger
+ /// guarantee than `ExactSizeIterator` provides, which could still report
a wrong length.
+ ///
+ /// # Panics
+ ///
+ /// Panics if the iterator does not report an upper bound on `size_hint()`.
#[inline]
pub unsafe fn from_trusted_len_iter<I, P>(iter: I) -> Self
where
P: std::borrow::Borrow<Option<<T as ArrowPrimitiveType>::Native>>,
- I: IntoIterator<Item = P>,
+ I: ExactSizeIterator<Item = P>,
Review Comment:
Yeah we just mentioned that as a side note in the last PR. I think it has
two advantages:
- Users have at least some help from the type system to avoid safety issues.
- It's consistent with BooleanArray
However, I am also happy with the old API.
--
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]
