itertools icon indicating copy to clipboard operation
itertools copied to clipboard

Implement `with_hasher` adaptors

Open 424ever opened this issue 1 year ago • 3 comments

Closes #998

Implementations for the following methods:

  • duplicates_with_hasher
  • duplicates_by_with_hasher
  • unique_with_hasher
  • unique_by_with_hasher
  • all_unique_with_hasher
  • into_group_map_with_hasher
  • into_group_map_by_with_hasher
  • into_grouping_map_with_hasher
  • into_grouping_map_by_with_hasher
  • counts_with_hasher
  • counts_by_with_hasher

424ever avatar Dec 23 '24 13:12 424ever

Two nits:

  • I'm a bit unhappy about duplicated tests. We can leave them as is and add a TODO that we'd ideally want to write all test cases once and apply them to both normal and _with_hasher functions automagically.

  • (Possibly over-nitpicky:) Should we rely on the fact that RandomState is the default BuildHasher or should we extract it so that std can do whatever it wants and we just adapt it? Could be done as follows:

    trait THashMap {
        type BuildHasher;
    }
    impl<K, V, S> THashMap for std::collections::HashMap<K, V, S> {
        type BuildHasher = S;
    }
    type StdDefaultBuildHasher = <std::collections::HashMap<(), ()> as THashMap>::BuildHasher;
    
    

@jswrenn Feel free to proceed as you deem appropriate.

phimuemue avatar Jan 04 '25 21:01 phimuemue

Codecov Report

Attention: Patch coverage is 99.55157% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.67%. Comparing base (6814180) to head (625f85f). Report is 134 commits behind head on master.

Files with missing lines Patch % Lines
src/grouping_map.rs 95.45% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1007      +/-   ##
==========================================
+ Coverage   94.38%   94.67%   +0.28%     
==========================================
  Files          48       50       +2     
  Lines        6665     7062     +397     
==========================================
+ Hits         6291     6686     +395     
- Misses        374      376       +2     

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

codecov[bot] avatar Jan 04 '25 21:01 codecov[bot]

  • Should we rely on the fact that RandomState is the default BuildHasher or should we extract it so that std can do whatever it wants and we just adapt it?

It would be a breaking change for Rust to change its default hasher, so we're fine to rely on RandomState directly.

jswrenn avatar Jan 15 '25 16:01 jswrenn