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

Make Intersperse[With] lazy #797

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Nov 6, 2023

Related to #792
To make Intersperse[With] adaptors lazy, I use a nested option which is what core::iter::Peekable uses internally.

EDITs: I certainly hope that it's okay to use a nested option in such case because I did a similar thing to make Product lazy (fixing cartesian_product and iproduct! un-laziness) and make CoalesceBy lazy (fixing 5 methods) that I'll publish once this PR is merged.

@@ -54,18 +54,17 @@ where
{
element: ElemF,
iter: Fuse<I>,
peek: Option<I::Item>,
peek: Option<Option<I::Item>>,
Copy link
Member

Choose a reason for hiding this comment

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

With the added nesting, could you just write a comment here about how the value of peek should be interpreted? (None, Some(None), Some(Some(...))?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. And I will do the same for Product and CoalesceBy before publishing their similar laziness fix.

@jswrenn jswrenn added this to the next milestone Nov 13, 2023
Similar to what is done by `core::iter::Peekable`, a nested option is now used.
@jswrenn jswrenn added this pull request to the merge queue Nov 13, 2023
Merged via the queue into rust-itertools:master with commit d7e6bab Nov 13, 2023
8 checks passed
@Philippe-Cholet Philippe-Cholet deleted the intersperse-laziness branch November 13, 2023 17:50
@jswrenn jswrenn mentioned this pull request Nov 14, 2023
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.

2 participants