Add Itertools::into_group_map#258
Conversation
|
This is welcome but should be named so that it does not conflict or confuse with potential equivalents of collect to hashmap under names like into_hashmap. |
| where Self: Iterator + Sized, | ||
| K: Hash + Eq, | ||
| F: Fn(&Self::Item) -> K | ||
| { |
There was a problem hiding this comment.
Naming aside (it is important too).
I would expect this to apply to iterators of pairs and use the first field as the key and the second as the value. Like HashMap::from_iter expects it. Then we don't have to worry about the diversity in calling mode required for extracting the key, be it by value (split the key out as a part of something) or by ref (clone/create a new key from a value).
This is a bit that we leave the model and conventions from C# behind (because it doesn't apply to Rust, we have ownership instead of ubiquitous reference taking), and we use the conventions and the model from Rust.
There was a problem hiding this comment.
I was actually thinking about just this :) haven't had much time to work on it this weekend but I was considering providing three different argument signatures:
- iterator of pairs,
- iterator with key function (i.e. the current version),
- iterator with both key and value functions.
Seems reasonable enough to only provide the first one. However, my initial reasoning for doing it as it currently is, was to match .group_by(). I can't see any reason why this function and that one would take different arguments. Maybe .group_by() should take a pair iterator too?
There was a problem hiding this comment.
Good point, they seem to be in about the same situation. Maybe they differ in that the group by key is often not kept around.
It seems they should both do the same thing, and if I had thought of that, I'd have picked to mimic group_by as well.
|
Changed name to |
| { | ||
| lookup::to_group_lookup(self) | ||
| } | ||
|
|
There was a problem hiding this comment.
This is an iterator "regular" method, not an adaptor, so it should be listed in next to the other methods.
|
Thanks. I'm going to suggest a name such as |
|
Edited; it's nice to say "Fulfills #92." but it is not what github prefers: https://help.github.com/articles/closing-issues-using-keywords/ |
|
Thanks for the solid work and thanks for using quickcheck tests. |
|
This is finally released in itertools 0.7.7. Thanks, I think it's a very nice new feature. |
2 participants
Fixes #92.
Create a Lookup from an Iterator, in a similar manner to C#'s ToLookup.
Returns a plain
HashMap, so no specialFromIteratoror.iter()implementations. My only concern is, if we do want to specialise this in future, it will require a new type and be a breaking change.