rluvaton commented on code in PR #8728:
URL: https://github.com/apache/arrow-rs/pull/8728#discussion_r2471016420
##########
arrow-array/src/iterator.rs:
##########
@@ -264,4 +269,924 @@ mod tests {
// check if ExactSizeIterator is implemented
let _ = array.iter().rposition(|opt_b| opt_b == Some(true));
}
+
+ trait SharedBetweenArrayIterAndSliceIter:
+ ExactSizeIterator<Item = Option<i32>> + DoubleEndedIterator<Item =
Option<i32>> + Clone
+ {
+ }
+ impl<T: Clone + ExactSizeIterator<Item = Option<i32>> +
DoubleEndedIterator<Item = Option<i32>>>
+ SharedBetweenArrayIterAndSliceIter for T
+ {
+ }
+
+ fn get_int32_iterator_cases() -> impl Iterator<Item = (Int32Array,
Vec<Option<i32>>)> {
+ let mut rng = StdRng::seed_from_u64(42);
+
+ let no_nulls_and_no_duplicates =
(0..10).map(Some).collect::<Vec<Option<i32>>>();
+ let no_nulls_random_values = (0..10)
+ .map(|_| rng.random::<i32>())
+ .map(Some)
+ .collect::<Vec<Option<i32>>>();
+
+ let all_nulls = (0..10).map(|_| None).collect::<Vec<Option<i32>>>();
+ let only_start_nulls = (0..10)
+ .map(|item| if item < 4 { None } else { Some(item) })
+ .collect::<Vec<Option<i32>>>();
+ let only_end_nulls = (0..10)
+ .map(|item| if item > 8 { None } else { Some(item) })
+ .collect::<Vec<Option<i32>>>();
+ let only_middle_nulls = (0..10)
+ .map(|item| {
+ if (4..=8).contains(&item) && rng.random_bool(0.9) {
+ None
+ } else {
+ Some(item)
+ }
+ })
+ .collect::<Vec<Option<i32>>>();
+ let random_values_with_random_nulls = (0..10)
+ .map(|_| {
+ if rng.random_bool(0.3) {
+ None
+ } else {
+ Some(rng.random::<i32>())
+ }
+ })
+ .collect::<Vec<Option<i32>>>();
+
+ let no_nulls_and_some_duplicates = (0..10)
+ .map(|item| item % 3)
+ .map(Some)
+ .collect::<Vec<Option<i32>>>();
+ let no_nulls_and_all_same_value =
+ (0..10).map(|_| 9).map(Some).collect::<Vec<Option<i32>>>();
+ let no_nulls_and_continues_duplicates = [0, 0, 0, 1, 1, 2, 2, 2, 2, 3]
+ .map(Some)
+ .into_iter()
+ .collect::<Vec<Option<i32>>>();
+
+ let single_null_and_no_duplicates = (0..10)
+ .map(|item| if item == 4 { None } else { Some(item) })
+ .collect::<Vec<Option<i32>>>();
+ let multiple_nulls_and_no_duplicates = (0..10)
+ .map(|item| if item % 3 == 2 { None } else { Some(item) })
+ .collect::<Vec<Option<i32>>>();
+ let continues_nulls_and_no_duplicates = [
+ Some(0),
+ Some(1),
+ None,
+ None,
+ Some(2),
+ Some(3),
+ None,
+ Some(4),
+ Some(5),
+ None,
+ ]
+ .into_iter()
+ .collect::<Vec<Option<i32>>>();
+
+ [
+ no_nulls_and_no_duplicates,
+ no_nulls_random_values,
+ no_nulls_and_some_duplicates,
+ no_nulls_and_all_same_value,
+ no_nulls_and_continues_duplicates,
+ all_nulls,
+ only_start_nulls,
+ only_end_nulls,
+ only_middle_nulls,
+ random_values_with_random_nulls,
+ single_null_and_no_duplicates,
+ multiple_nulls_and_no_duplicates,
+ continues_nulls_and_no_duplicates,
+ ]
+ .map(|case| (Int32Array::from(case.clone()), case))
+ .into_iter()
+ }
+
+ trait SetupIter {
+ fn setup<I: SharedBetweenArrayIterAndSliceIter>(&self, iter: &mut I);
+ }
+
+ struct NoSetup;
+ impl SetupIter for NoSetup {
+ fn setup<I: SharedBetweenArrayIterAndSliceIter>(&self, _iter: &mut I) {
+ // none
+ }
+ }
+
+ fn setup_and_assert_cases(
+ setup_iterator: impl SetupIter,
+ assert_fn: impl Fn(ArrayIter<&Int32Array>, Copied<Iter<Option<i32>>>),
+ ) {
+ for (array, source) in get_int32_iterator_cases() {
+ let mut actual = ArrayIter::new(&array);
+ let mut expected = source.iter().copied();
+
+ setup_iterator.setup(&mut actual);
+ setup_iterator.setup(&mut expected);
+
+ assert_fn(actual, expected);
+ }
+ }
+
+ /// Trait representing an operation on a [`ArrayIter`]
+ /// that can be compared against a slice iterator
+ ///
+ /// this is for consuming operations (e.g. `count`, `last`, etc)
+ trait ConsumingArrayIteratorOp {
+ /// What the operation returns (e.g. Option<i32> for last, usize for
count, etc)
+ type Output: PartialEq + Debug;
+
+ /// The name of the operation, used for error messages
+ fn name(&self) -> String;
+
+ /// Get the value of the operation for the provided iterator
+ /// This will be either a [`ArrayIter`] or a slice iterator to make
sure they produce the same result
+ fn get_value<T: SharedBetweenArrayIterAndSliceIter>(&self, iter: T) ->
Self::Output;
+ }
+
+ /// Trait representing an operation on a [`ArrayIter`]
+ /// that can be compared against a slice iterator.
+ ///
+ /// This is for mutating operations (e.g. `position`, `any`, `find`, etc)
+ trait MutatingArrayIteratorOp {
+ /// What the operation returns (e.g. Option<i32> for last, usize for
count, etc)
+ type Output: PartialEq + Debug;
+
+ /// The name of the operation, used for error messages
+ fn name(&self) -> String;
+
+ /// Get the value of the operation for the provided iterator
+ /// This will be either a [`ArrayIter`] or a slice iterator to make
sure they produce the same result
+ fn get_value<T: SharedBetweenArrayIterAndSliceIter>(&self, iter: &mut
T) -> Self::Output;
+ }
+
+ /// Helper function that will assert that the provided operation
+ /// produces the same result for both [`ArrayIter`] and slice iterator
+ /// under various consumption patterns (e.g. some calls to
next/next_back/consume_all/etc)
+ fn assert_array_iterator_cases<O: ConsumingArrayIteratorOp>(o: O) {
+ setup_and_assert_cases(NoSetup, |actual, expected| {
+ let current_iterator_values: Vec<Option<i32>> =
expected.clone().collect();
Review Comment:
The only difference is the error message, so I moved the assertion to the
setup and assert function and added a description for the setup
--
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]