rust-itertools / itertools

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

New `Itertools::tail` #899

Closed Philippe-Cholet closed 4 months ago

Philippe-Cholet commented 4 months ago

Hi, after 80 merged PRs fixing, modifying, testing, benchmarking, specializing or whatever really... I just checked this is my first PR where I actually add a "brand new" feature myself. 🤣

PS from the future: The discussion also lead me to contribute a small optimization inside VecDeque::pop_{back,front} that will be released in rust 1.79.0. 😎

codecov[bot] commented 4 months ago

Codecov Report

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

Project coverage is 94.50%. Comparing base (6814180) to head (b51f26a). Report is 36 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #899 +/- ## ========================================== + Coverage 94.38% 94.50% +0.11% ========================================== Files 48 48 Lines 6665 6807 +142 ========================================== + Hits 6291 6433 +142 Misses 374 374 ```

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

Philippe-Cholet commented 4 months ago

I wonder if libcore would want a Iterator::tail<const N: usize> (not as straightforward as this implementation in case the iterator generate less than N items but it's possible) in which case name my method Itertools::tail(n: usize) would clash.

But we could then wonder about k_smallest and its variants where [_; N] is enough storage.

Philippe-Cholet commented 4 months ago

I fused the iterator instead of checking data.len() == n (same as recent fix #900).

Philippe-Cholet commented 4 months ago

I polished the commits and added @scottmcm as co-author. I particularly like his idea of skipping the starting part of the iterator.

Is there a better name than tail? (see this) If not then I think this can be merged.

Note for myself: File a rust issue for a possible method more optimized than .pop_front()+.push_back(_), compared to my vector implementation (see this).

EDIT: Discussion there.