rust-itertools / itertools

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

feat(`FilterMapOk`): implement `DoubleEndedIterator` #950

Closed Xenira closed 6 months ago

Xenira commented 6 months ago

DoubleEndedIterator implementation for FilterMapOk analog to #948.

Hope rev() is the right thing here, as rfind_map does not exist (#949).

Refs: #947

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.52%. Comparing base (6814180) to head (aa547c6). Report is 98 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #950 +/- ## ========================================== + Coverage 94.38% 94.52% +0.13% ========================================== Files 48 49 +1 Lines 6665 7046 +381 ========================================== + Hits 6291 6660 +369 - Misses 374 386 +12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Xenira commented 6 months ago

You did not need to make a merge commit.

Wanted to rebase, but messed up. Like my pr's to be up to date :)

Philippe-Cholet commented 6 months ago

No prob, the commit history is not that clean anyway.

scottmcm commented 6 months ago

Hmm, maybe the other option would be .filter_map(p).next_back(), inspired by https://github.com/rust-lang/rust/pull/49098 ?

The rev is probably fine, though, so probably doesn't need to be changed.

Xenira commented 6 months ago

Benchmarked against the filter_map variant and its mostly in noise threshold. (2 cases 2% faster but I guess, thats because different base load on my system).

The rev() is more readable though.