Skip to content

Add Iterator::zip_longest. #19283

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 2 commits into from
Closed

Conversation

SimonSapin
Copy link
Contributor

Like Iterator::zip, but the returned iterator is as long as the longest of the input iterators rather than shortest, and yields (Option<A>, Option<B>) (which can not both be None) rather than (A, B).

An alternative could be to yield a new EitherOrBoth (?) enum that is either Both(A, B), Left(A), or Right(B).

Precedent: https://docs.python.org/library/itertools.html#itertools.izip_longest

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

@tikue
Copy link
Contributor

tikue commented Dec 3, 2014

I like the idea of EitherOrBoth better than (Option<A>, Option<B>). As it is now, I worry that usage would require (None, None) => unreachable!() in a lot of places.

@SimonSapin
Copy link
Contributor Author

I’m slightly reluctant since EitherOrBoth sounds a lot like Either, which was removed: #9157

@tikue
Copy link
Contributor

tikue commented Dec 3, 2014

I don't have a preference on the name, but I think the functionality of EitherOrBoth leverages the type system better. Perhaps a name like ZippedOneOrBoth is easier to reason about? In any case, I don't think the name is as bad as Either was, because that was a general utility that should in most cases be replaced with a more specific enum and was otherwise redundant with Result, whereas this would be an enum specific to the Iterator's zip_longest method.

@SimonSapin
Copy link
Contributor Author

It’s not so much about the name, it’s an a very similar type used to exist in std and was removed. @aturon, what do you think?

@thestinger
Copy link
Contributor

I don't see what Either being removed has to do with this. The rationale for removing was that custom sum types are almost always more appropriate because they can communicate more information - although they lose the ability to have many useful convenience methods out of the box, at least without making deriving for it.

@aturon
Copy link
Member

aturon commented Dec 4, 2014

I'm personally fine with accepting this on an #[experimental] basis, and seeing whether we get enough usage to warrant a permanent place in std. I'm also in favor of a custom enum; EitherOrBoth is fine by me. (It could always have a convenience method to turn it into a pair of options).

@aturon
Copy link
Member

aturon commented Dec 4, 2014

cc @alexcrichton

Like `Iterator::zip`, but the returned iterator is as long as the longest
of the input iterators rather than shortest,
and yields `(Option<A>, Option<B>)` (which can not both be `None`)
rather than `(A, B)`.

An alternative could be to yield a new `EitherOrBoth` (?) enum
that is either `Both(A, B)`, `Left(A)`, or `Right(B)`.

Precedent: https://docs.python.org/library/itertools.html#itertools.izip_longest
@SimonSapin
Copy link
Contributor Author

Rebased and switched to EitherOrBoth.

I’ve also published this on crates.io:

https://crates.io/crates/zip-longest
https://github.com/SimonSapin/rust-std-candidates

@aturon
Copy link
Member

aturon commented Dec 8, 2014

@SimonSapin The itertools crate might actually be a good place for this to go for the moment. I can easily imaging us importing adapters from that crate over time, as they prove their worth.

@SimonSapin
Copy link
Contributor Author

@aturon: Ok. Feel free to postpone this PR to after 1.0.

Whoever maintains the itertools crate: feel free to copy or re-export the zip_longest crate.

@aturon
Copy link
Member

aturon commented Dec 8, 2014

Whoever maintains the itertools crate: feel free to copy or re-export the zip_longest crate.

Paging @bluss!

I'm going to close this for now. Thanks for your understanding, @SimonSapin.

@aturon aturon closed this Dec 8, 2014
bluss pushed a commit to rust-itertools/itertools that referenced this pull request Dec 9, 2014
ZipLongest originally written by SimonSapin,
and dedicated to itertools rust-lang/rust#19283
@bluss
Copy link
Member

bluss commented Dec 9, 2014

Done. :-)

http://bluss.github.io/rust-itertools/doc/itertools/trait.Itertools.html#method.zip_longest

itertools is using the same license as Rust itself, for the very reason that it should be easy to lift things back and forth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants