Skip to content

Added Range iterator #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
37 changes: 37 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ mod combinations;
mod combinations_with_replacement;
mod exactly_one_err;
mod diff;
mod range;
mod format;
#[cfg(feature = "use_std")]
mod group_map;
Expand Down Expand Up @@ -396,6 +397,42 @@ pub trait Itertools : Iterator {
intersperse::intersperse(self, element)
}

/// Limits an iterator to a given range.
/// Similar to [`Iterator::skip`] and [`Iterator::take`],
/// but some may consider it to be more readable.
///
/// # Examples
///
/// ```
/// use itertools::Itertools;
///
/// let vec = vec![3, 1, 4, 1, 5];
///
/// let mut range: Vec<_> =
/// vec.iter().range(1..=3).copied().collect();
/// assert_eq!(&range, &[1, 4, 1]);
///
/// // It works with other types of ranges, too
/// range = vec.iter().range(..2).copied().collect();
/// assert_eq!(&range, &[3, 1]);
///
/// range = vec.iter().range(0..1).copied().collect();
/// assert_eq!(&range, &[3]);
///
/// range = vec.iter().range(2..).copied().collect();
/// assert_eq!(&range, &[4, 1, 5]);
///
/// range = vec.iter().range(..).copied().collect();
/// assert_eq!(range, vec);
/// ```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test about RangeToInclusive (such as ..=2) yet, add one or codecov will rightfully complain.

fn range<R>(self, range: R)
-> R::IterTo
where R: range::IntoRangeIter<Self>,
Self: Sized
{
range::range(self, range)
}

/// Create an iterator which iterates over both this and the specified
/// iterator simultaneously, yielding pairs of two optional elements.
///
Expand Down
86 changes: 86 additions & 0 deletions src/range.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use core::ops::{ Range, RangeTo, RangeFrom, RangeFull, RangeInclusive, RangeToInclusive };
use core::iter::{Skip, Take};
use crate::Itertools;

/// Used by the ``range`` function to know which iterator
/// to turn different ranges into.
pub trait IntoRangeIter<IterFrom> {
type IterTo;

fn into_range_iter(self, from: IterFrom) -> Self::IterTo;
}

impl<I> IntoRangeIter<I> for Range<usize>
where I: Iterator
{
type IterTo = Take<Skip<I>>;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter.skip(self.start)
.take(self.end.saturating_sub(self.start))
}
}

impl<I> IntoRangeIter<I> for RangeInclusive<usize>
where I: Iterator
{
type IterTo = Take<Skip<I>>;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter.skip(*self.start())
.take(
(1 + *self.end())
.saturating_sub(*self.start())
)
}
}

impl<I> IntoRangeIter<I> for RangeTo<usize>
where I: Iterator
{
type IterTo = Take<I>;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter.take(self.end)
}
}

impl<I> IntoRangeIter<I> for RangeToInclusive<usize>
where I: Iterator
{
type IterTo = Take<I>;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter.take(self.end + 1)
}
}

impl<I> IntoRangeIter<I> for RangeFrom<usize>
where I: Iterator
{
type IterTo = Skip<I>;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter.skip(self.start)
}
}

impl<I> IntoRangeIter<I> for RangeFull
where I: Iterator
{
type IterTo = I;

fn into_range_iter(self, iter: I) -> Self::IterTo {
iter
}
}

/// Limits an iterator to a range. See [`Itertools::range`]
/// for more information.
pub fn range<I, R>(iter: I, range: R)
-> R::IterTo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of exploiting existing iterator implementations. Three questions:

  • Should we introduce a type alias RangeIterator<R, I> so that our public interface does not expose that the return type is realized as an associated type?

  • I guess at some point users want to write:

    fn doit(r: impl std::ops::RangeBounds) {
     give_me_usize_iterator().range(r); // somewhere within the function
    }
    

    This - as far as I can see - does not work because range expects an IntoRangeIter, so the user probably goes ahead and tries:

    fn doit(r: impl IntoRangeIter) {
     give_me_usize_iterator().range(r); // somewhere within the function
    }
    

    But if I am not mistaken this will not work either, because IntoRangeIter requires to specify an iterator upfront. Is it possible to implement IntoRangeIter in a way that does not require the user to specify the iterator?

  • Should we try to inform the type system that each IntoRangeIter also satisfies std::ops::RangeBounds so that users can write

    fn doit(r: impl IntoRangeIter<(possibly iter type)> {
      give_me_iter().range(r.clone());
      vec![1,2,3,4,5].drain(r);
    }
    

    without having to specify the (imho opinion) implied RangeBounds trait bound?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the TraitBounds -> IntoRangeIter conversion can work, because IntoRangeIter type is implement specifically for each TraitBounds type, so implementing it for all TraitBounds types would be a conflict. I'm not sure if rust team is working on prioritized implementations, but if it's possible to somehow say "use these implementations, or this other even more generic one if the specific ones are not possible".

IntoIterRange implementing RangeBounds is definitely something we should add.

And for some reason I had the idea that trait methods couldn't have generics, but that doesn't seem to be the case, so we shouldn't need to pass around the iterator information in the trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I'll see if I can figure out that type alias thing. I am not too used to working with some of these things, I have stuck in my little rust box and not gone into other places too much, but it's great that I am now!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that maybe we should rename "IntoRangeIter" to "IterRangeBounds", because I feel like that makes more sense. The bounds aren't the thing that turn into an iterator after all, they just are a RangeBounds that work on an iterator

where I: IntoIterator,
R: IntoRangeIter<I::IntoIter>
{
range.into_range_iter(iter.into_iter())
}