Skip to content
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

Implement interleave_shortest() #445

Merged
merged 2 commits into from Sep 25, 2017

Conversation

Projects
None yet
2 participants
@laumann
Copy link
Contributor

laumann commented Sep 22, 2017

Add interleave_shortest() to IndexedParallelIterator that acts like
interleave, except it terminates as soon as one of the iterators is
exhausted.

The other part of/Fixes #384

Implement interleave_shortest()
Add interleave_shortest() to IndexedParallelIterator that acts like
interleave, except it terminates as soon as one of the iterators is
exhausted.

The other part of/Fixes #384
@laumann

This comment has been minimized.

Copy link
Contributor Author

laumann commented Sep 22, 2017

Just wondering, why does the size_hint() implementation for InterleaveShortest in itertools look so complicated? Makes me wonder if I'm missing something here...

@cuviper

This comment has been minimized.

Copy link
Member

cuviper commented Sep 23, 2017

Looks good! Just one thing, can you add this to tests/compile-fail/must_use.rs? I forgot this for interleave too.

Just wondering, why does the size_hint() implementation for InterleaveShortest in itertools look so complicated?

Probably because itertools doesn't require them to be ExactSizeIterator, so it's going through hoops to combine the min/max limits from the two iterators. We're also lax about overflow when two iterators combine to overflow usize. Actually, let's improve that. Interleave can copy what Chain does, and while we're here, it should also pass through min_len and max_len. [nevermind, you did those already.]

interleave_shortest: Review changes
 - Add `interleave` and `interleave_shortest` to
   tests/compile-fail/must_use.rs
 - Change implementation of `len()` for Interleave to use
   `checked_add(..).expect(..)` to explicitly handle overflow (like
   Chain)
@cuviper

This comment has been minimized.

Copy link
Member

cuviper commented Sep 25, 2017

Thanks!

@cuviper cuviper merged commit 6cde670 into rayon-rs:master Sep 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.