itertools icon indicating copy to clipboard operation
itertools copied to clipboard

Add `.get(range)`

Open Philippe-Cholet opened this issue 1 year ago • 1 comments

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
    impl<L: Sealed, R: Sealed> Sealed for either::Either<L, R> {}


impl<I, L, R> IteratorIndex<I> for Either<L, R>
where
    I: Iterator,
    L: IteratorIndex<I>,
    R: IteratorIndex<I>,
{
    type Output = Either<<L as IteratorIndex<I>>::Output, <R as IteratorIndex<I>>::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]);

Philippe-Cholet avatar Mar 01 '24 11:03 Philippe-Cholet

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
@@            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.

codecov[bot] avatar Mar 01 '24 11:03 codecov[bot]

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.

Philippe-Cholet avatar May 02 '24 09:05 Philippe-Cholet