rust-itertools / itertools

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

`Peekable` Adapters: `peeking_take_while`, `peek_map`, `peeking_fold_while` and `try_` #979

Open G-M0N3Y-2503 opened 3 months ago

G-M0N3Y-2503 commented 3 months ago

Note: I am primarily seeking feedback rather than proposing at this stage.

Peekable

As I see it peekable (Existing in std) facilitates two concepts, the ability to preview an item in an iterator and to optionally consume said item. Though it's not a limitation, consumption typically occurs after and/or with some information from a previewed item (&Self::Item). It is also notable that these only hold true at the point of the Peekable adapter, that is to say, at the point of a previous adapter, at least one of the peeked for Peekable and more for PeekingNext (Existing Trait) items will have been consumed.

Existing Extensions

The peeking_take_while (Existing in itertools) and similar take_while_ref (Existing in itertools) consume depending on a predicate with the information from the previewed item (&Self::Item), though take_while_ref isn't technically peeking. For the output of these adapters, they output the owned item (Self::Item) when it has been accepted via the predicate otherwise none. Generally, the iterator will continue not to output items, as the predicate is fixed at the creation of the iterator, unless interior mutability is used.

Remaining Problem Space

As I see it, the problem space doesn't have good solutions for the following (conceptually accumulating) examples.

  1. maping with information from or computed with the previewed item (&Self::Item).
    • i.e. optimising for:
      iter.peekable().peeking_take_while(|item| compute(item).is_ok()).map(|item| compute(&item).unwrap().to_string())
  2. Notifying that unaccepted items remain, like try_for_each but with Peekable.
    • i.e. simplifying the following:
      loop {
        for item in peekable
            .peeking_take_while(|item: &i32| compute(item).is_ok())
            .map(|item| compute(&item).unwrap().to_string())
        {
            accepted.push(item);
        }
        if let Some(item) = peekable.peek() {
            match compute(item) {
                Ok(_) => unreachable!(),
                Err("count too high") => {
                    *count.borrow_mut() -= 1;
                    continue;
                }
                Err(_) => todo!(),
            }
        }
        break;
      }
  3. Adapter reuse, The peeking_take_while adapter can only be reused when you don't need any of the unaccepted items.
    • i.e. the following doesn't compile as it mutably borrows peekable once for the peeking_take_while adapter and again for the following .peek().
      let mut take_while = peekable
        .peeking_take_while(|item: &i32| compute(item).is_ok())
        .map(|item| compute(&item).unwrap().to_string());
      loop {
        for item in &mut take_while {
            accepted.push(item);
        }
        if let Some(item) = peekable.peek() {
            match compute(item) {
                Ok(_) => unreachable!(),
                Err("count too high") => {
                    *count.borrow_mut() -= 1;
                    continue;
                }
                Err(_) => todo!(),
            }
        }
        break;
      }

Potential Peekable Adapters

With this in mind, the most general solution for the entire problem space might look something like the following:

fn peeking_map<Accept, Map, Output>(&mut self, f: Accept) -> PeekingMap<'_, Self, Accept>
where
    Self: Sized,
    Map: FnOnce(Self::Item) -> Output,
    Accept: FnMut(&mut Self::Item) -> ControlFlow<Option<Output>, Map>,
{}

with the above examples then looking like this:

let mut map = iter.peeking_map(|item| match compute(item) {
    Ok(value) => ControlFlow::Continue(move |_owned| Ok(value.to_string())),
    Err(err) => ControlFlow::Break(Some(Err(err))),
});
loop {
    for item in &mut map {
        match item {
            Ok(item) => {
                accepted.push(item);
                continue;
            }
            Err("count too high") => {
                *count.borrow_mut() -= 1;
                continue;
            }
            Err(_) => todo!(),
        }
    }
    break;
}

and the peeking_take_while could then be emulated by this:

let take_while = iter.peeking_map(|item| {
    if compute(item).is_ok() {
        ControlFlow::Continue(|owned| owned)
    } else {
        ControlFlow::Break(None)
    }
});

Related sources

peekable (Existing in std) peek_map peeking_take_while (Existing in itertools)

hacked together compile only implimentation ```rust pub struct PeekingMap<'a, I, F> where I: Iterator + 'a, { iter: Peekable<&'a mut I>, f: F, } impl<'a, I, Accept, Map, Output> Iterator for PeekingMap<'a, I, Accept> where I: Iterator, Map: FnOnce(I::Item) -> Output, Accept: FnMut(&mut I::Item) -> ControlFlow, Map>, { type Item = Output; fn next(&mut self) -> Option { match (self.f)(self.iter.peek_mut()?) { ControlFlow::Continue(map) => Some(map(self.iter.next()?)), ControlFlow::Break(res) => res, } } } trait Tmp: Itertools { fn peeking_map(&mut self, f: Accept) -> PeekingMap<'_, Self, Accept> where Self: Sized, Map: FnOnce(Self::Item) -> Output, Accept: FnMut(&mut Self::Item) -> ControlFlow, Map>, { PeekingMap { iter: self.peekable(), f, } } } fn test(mut iter: impl Tmp) -> Result<(), &'static str> { let count = RefCell::new(6i32); let compute = |item: &i32| { let count = *count.borrow(); if &count < item { Ok(item - count) } else { Err("count too high") } }; let mut accepted = vec![]; let take_while = iter.peeking_map(|item| { if compute(item).is_ok() { ControlFlow::Continue(|owned| owned) } else { ControlFlow::Break(None) } }); let mut map = iter.peeking_map(|item| match compute(item) { Ok(value) => ControlFlow::Continue(move |_owned| Ok(value.to_string())), Err(err) => ControlFlow::Break(Some(Err(err))), }); loop { for item in &mut map { match item { Ok(item) => { accepted.push(item); continue; } Err("count too high") => { *count.borrow_mut() -= 1; continue; } Err(_) => todo!(), } } break; } Ok(()) } ```
G-M0N3Y-2503 commented 2 months ago

https://github.com/G-M0N3Y-2503/itertools/commit/98139cf05bf8f24c8127906f476638d15988105d

G-M0N3Y-2503 commented 2 months ago

After playing around with a peeking_map() with subsequent adapters, I found that subsequent adapters would require the burden of maintaining the knowledge of whether a value was taken or not, which may vary depending on Output. It could probably be made nicer with a try_next() but that would introduce branches for all the subsequent adapters. The only real way to manage actual short-circuiting that I can see would be within a single code block. So regarding iterators that would be the try_fold.

However, there are a handful of issues with the current peeking_fold_while:

  1. It doesn't use the experimental std::ops::Try which would allow for non-eronious short-circuiting, though this is just for readability.
  2. It restricts the implementation for PutBack<I> by unnecessarily constraining it to an immutable reference, where the owned value could be necessary for some.
  3. With only a reference, a fold may not be able to be completed only validated, It could need the owned value and the implementation for Peekable<I> currently discards this.
dvdsk commented 2 weeks ago

I would like to add peeking_skip_while.

My use case: finding the last item smaller then x and the first item larger then x such that I can generate an interpolated item.

With peeking_skip_while that would look like this:

data.iter()
    .peekable()
    .peeking_skip_while(|(x, _)| *x < pos)
    .next_tuple() // yields x1, x2 where x1 < x > x2
    .ok_or(TooInaccurate)
    .inspect(|v| eprintln!("v: {v:?}, pos: {pos}"))
    .map(|((a, ay), (b, by))| if pos - a < b - pos { (a, ay) } else { (b, by) })
    .and_then(|(x, y)| {
        if (pos - x).abs() <= max_deviation {
            Ok(*y)
        } else {
            Err(TooInaccurate)
        }
    })