rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.64k stars 299 forks source link

Add `.get(range)` #891

Closed Philippe-Cholet closed 2 months ago

Philippe-Cholet commented 4 months ago

Closes #447.

So much changes happened since this work was initiated that rebase on top of master seems nothing less than a nightmare to me (I know, I tried). I therefore looked at the git differences, committed those (after formatting) and credited @wod3 and @TrolledWoods for their respective commits. Then I fixed my own review and a bit more.


I thought I would additionally implement IteratorIndex for Either<range, range> (and Box<dyn range> but I could not) but doing so would later result in a nasty breaking change if this was removed after an eventual inclusion to Iterator.

Details about either ```rust impl Sealed for either::Either {} impl IteratorIndex for Either where I: Iterator, L: IteratorIndex, R: IteratorIndex, { type Output = Either<>::Output, >::Output>; fn index(self, iter: I) -> Self::Output { // Could be more succinct with `map_either_with` but it would require to bump `either` version. match self { Either::Left(left) => Either::Left(left.index(iter)), Either::Right(right) => Either::Right(right.index(iter)), } } } /// // It works nicely with either. /// use itertools::Either; /// /// let mut either_range = Either::Left(0..1); /// range = vec.iter().get(either_range).copied().collect(); /// assert_eq!(&range, &[3]); /// /// either_range = Either::Right(2..); /// range = vec.iter().get(either_range).copied().collect(); /// assert_eq!(&range, &[4, 1, 5]); ```
codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.57%. Comparing base (6814180) to head (3f49188). Report is 79 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #891 +/- ## ========================================== + Coverage 94.38% 94.57% +0.18% ========================================== Files 48 49 +1 Lines 6665 7002 +337 ========================================== + Hits 6291 6622 +331 - Misses 374 380 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Philippe-Cholet commented 2 months ago

I rebased on master and added 4 commits:

  1. Is there a reason to prefer Take<Skip> over Skip<Take>? As Skip<Take> is simpler defined!
  2. I don't see how we should really handle a range that includes usize::MAX other than with panic.
  3. Clarified when the resulting iterator is ExactSizeIterator and/or DoubleEndedIterator.
  4. fix 2nd commit: .get(1..=usize::MAX) should be fine (EDIT: tested).

And removed get as a free function.