rust-itertools / itertools

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

`.try_collect()` for `Iterator<Option>` #951

Open amab8901 opened 4 months ago

amab8901 commented 4 months ago

.try_collect() is for turning Iterator<Result<T>> into Result<Iterator<T>>. Can you create an equivalent method that turns Iterator<Option<T>> into Option<Iterator<T>>? Or perhaps implement it as a generic, as part of try_collect()?

The use case is for creating programs that never crash at runtime (where error conditions are passed to the logs, and Result converted to Option, and program is continuing without interruption).

Philippe-Cholet commented 4 months ago

try_collect does not give Result<Iterator> but Result<Vec<...>> (or another container than Vec).

But in #844 we discussed fallible iterators and we might generalize (amongst other things) try_collect (which conflict with nightly Rust) to maybe_collect that would solve your use case.

amab8901 commented 4 months ago

you're right, I missed the Vec part. Here is an example code that would be nice if it worked:

use itertools::Itertools;

fn main() {
    let string: String = [1_u8, 20_u8, 200_u8]
        .iter()
        .map(|u_8| {
            let option_char = if (32..127).contains(u_8) || (160..=255).contains(u_8) {
                let character = char::from(*u_8);
                Some(character)
            } else {
                println!("Error: must be ascii character: `{u_8}`"); // actually tracing::error!() but I wanna keep the example simple
                None
            };

            option_char
        })
        .try_collect()?;

    unreachable!();
}
Philippe-Cholet commented 4 months ago

Our maybe_collect would merely be a stable Iterator::try_collect, so maybe_collect would eventually be deprecated in favor of it.

scottmcm commented 4 months ago

Can you create an equivalent method that turns Iterator<Option<T>> into Option<Iterator<T>>?

It's literally impossible to make such a method that's lazy.

And if you're fine with Iterator<Option<T>>Option<Vec<T>>, that already exists as collect.

(where error conditions are passed to the logs, and Result converted to Option, and program is continuing without interruption)

Perhaps one of the patterns in https://doc.rust-lang.org/stable/rust-by-example/error/iter_result.html#iterating-over-results would be helpful for you?

Philippe-Cholet commented 4 months ago

try_collect()? is merely a shortcut for .collect::<Result<_, _>>()?

So yeah, there is .collect::<Option<_>>()? here.

amab8901 commented 4 months ago

I ended up replacing None with Error inside the map, then I used try_collect(), and then I did .map_err() to produce log, then I did .ok()

Philippe-Cholet commented 4 months ago

That seems more hacky than .collect::<Option<_>>()?.