itertools icon indicating copy to clipboard operation
itertools copied to clipboard

New `Itertools::tail`

Open Philippe-Cholet opened this issue 1 year ago • 4 comments

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. 🤣


Note that all cases consume the iterator (even when n=0).

It could alternatively return a vector and not VecIntoIter but most of our methods do this (and immediately collect to a vector won't reallocate).

I don't think we should write a non-lazy head method (.take(n) is a lazy one) but it would be easy: .take(n).collect_vec().into_iter().

Philippe-Cholet avatar Mar 08 '24 16:03 Philippe-Cholet

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
@@            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.

codecov[bot] avatar Mar 08 '24 16:03 codecov[bot]

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 avatar Mar 09 '24 07:03 Philippe-Cholet

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

Philippe-Cholet avatar Mar 14 '24 13:03 Philippe-Cholet

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.

Philippe-Cholet avatar Mar 16 '24 09:03 Philippe-Cholet