Skip to content

Conversation

@Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Dec 8, 2023

Following #813 (comment), with this PR, Positions now uses Enumerate internally.

  • next and next_back are now based on find_map instead of "while-next" loops. It would be useful performance-wise for iterators specializing try_[r]fold. I intend to do the same for other while-next loops (where benchmarks show times decrease, up to -20%).
  • Stable benchmarks here.
  • Doing this, I first made a silly mistake. I therefore add a test that would have helped me, and fix a little error in the documentation (it could alternatively use filter_map to avoid * copy).

`Positions` is tested to be fused, for specialized methods and has two doctests. No other test.
While I was adding `Enumerate` in `Positions` internals, running `cargo test positions` told me `rfold` was wrong. The silly mistake was on `next_back` instead and this test would have helped me.
Plus, I noticed that the documentation was just a bit wrong.
Copy link
Member

@phimuemue phimuemue left a comment

Choose a reason for hiding this comment

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

Thank you, @Philippe-Cholet

@Philippe-Cholet Philippe-Cholet added this pull request to the merge queue Dec 10, 2023
Merged via the queue into rust-itertools:master with commit cfb2774 Dec 10, 2023
@Philippe-Cholet Philippe-Cholet deleted the positions-with-enumerate branch December 10, 2023 18:07
@Philippe-Cholet Philippe-Cholet added this to the next milestone Dec 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