Project

General

Profile

Feature #12512

Import Hash#transform_values and its destructive version from ActiveSupport

Added by mrkn (Kenta Murata) 11 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:76095]

Description

I think value transformation is a fundamental feature of Hash.


Related issues

Related to Ruby trunk - Feature #7793: New methods on Hash Assigned
Related to Ruby trunk - Feature #6669: A method like Hash#map but returns hash Feedback 06/30/2012
Related to Ruby trunk - Feature #8951: Please add a hash-to-hash alternative of the map method to Hash Open 09/25/2013
Related to Ruby trunk - Feature #9635: Map a hash directly to a hash Open 03/13/2014
Related to Ruby trunk - Feature #9970: Add `Hash#map_keys` and `Hash#map_values` Open
Related to Ruby trunk - Feature #10208: Passing block to Enumerable#to_h Feedback

Associated revisions

Revision 55847
Added by mrkn (Kenta Murata) 10 months ago

hash.c: implement Hash#map_v and Hash#map_v!

  • hash.c (rb_hash_map_v, rb_hash_map_v_bang): impelement Hash#map_v and
    Hash#map_v! [Feature #12512]

  • test/ruby/test_hash.rb: add tests for above change.

Revision 55847
Added by mrkn (Kenta Murata) 10 months ago

hash.c: implement Hash#map_v and Hash#map_v!

  • hash.c (rb_hash_map_v, rb_hash_map_v_bang): impelement Hash#map_v and
    Hash#map_v! [Feature #12512]

  • test/ruby/test_hash.rb: add tests for above change.

Revision 56099
Added by mrkn (Kenta Murata) 9 months ago

hash.c: map_v -> transform_values

  • hash.c (rb_hash_transform_values, rb_hash_transform_values_bang):
    Rename map_v to transform_values.
    [Feature #12512]

  • test/ruby/test_hash.rb: ditto.

Revision 56099
Added by mrkn (Kenta Murata) 9 months ago

hash.c: map_v -> transform_values

  • hash.c (rb_hash_transform_values, rb_hash_transform_values_bang):
    Rename map_v to transform_values.
    [Feature #12512]

  • test/ruby/test_hash.rb: ditto.

History

#1 Updated by mrkn (Kenta Murata) 11 months ago

#2 [ruby-core:76096] Updated by mrkn (Kenta Murata) 11 months ago

  • Description updated (diff)

#3 [ruby-core:76097] Updated by mrkn (Kenta Murata) 11 months ago

  • Description updated (diff)

#4 Updated by shyouhei (Shyouhei Urabe) 11 months ago

  • Related to Feature #6669: A method like Hash#map but returns hash added

#5 Updated by shyouhei (Shyouhei Urabe) 11 months ago

  • Related to Feature #8951: Please add a hash-to-hash alternative of the map method to Hash added

#6 Updated by shyouhei (Shyouhei Urabe) 11 months ago

#7 Updated by shyouhei (Shyouhei Urabe) 11 months ago

  • Related to Feature #9970: Add `Hash#map_keys` and `Hash#map_values` added

#8 [ruby-core:76100] Updated by shyouhei (Shyouhei Urabe) 11 months ago

CF: https://github.com/rails/rails/pull/15819

I believe no one is against the feature itself.

#9 [ruby-core:76461] Updated by shyouhei (Shyouhei Urabe) 10 months ago

We looked at this issue at yesterday's developer meeting and no one there was against this API, including the value passed to its block, but not the name. This name is not rejected, though. The word "transform" had no usage in core before so we were wary about introducing it.

#10 [ruby-core:76770] Updated by mrkn (Kenta Murata) 10 months ago

I don't persist with the name transform_value.
map_v or map_values are also acceptable for me.

#11 [ruby-core:76780] Updated by matz (Yukihiro Matsumoto) 10 months ago

#map_v accepted.

Matz.

#12 [ruby-core:76781] Updated by mrkn (Kenta Murata) 10 months ago

  • Status changed from Open to Assigned
  • Assignee changed from matz (Yukihiro Matsumoto) to mrkn (Kenta Murata)

#13 Updated by mrkn (Kenta Murata) 10 months ago

  • Status changed from Assigned to Closed

Applied in changeset r55847.


hash.c: implement Hash#map_v and Hash#map_v!

  • hash.c (rb_hash_map_v, rb_hash_map_v_bang): impelement Hash#map_v and
    Hash#map_v! [Feature #12512]

  • test/ruby/test_hash.rb: add tests for above change.

#14 [ruby-core:76801] Updated by janfri (Jan Friedrich) 10 months ago

Yukihiro Matsumoto wrote:

#map_v accepted.

At first I thought it was an "adapted form" of Hash#map like Enumerable#grep_v is an "inverted" Enumerable#grep. Maybe the "_v" could be confusing for other people?

Why not Hash#map_values - we have Enumerable#each_with_index, Enumerable#each_with_object, Hash#keys, Hash#values, Hash#values_at and other "full forms" of method names?

#15 [ruby-core:76819] Updated by Eregon (Benoit Daloze) 10 months ago

Yukihiro Matsumoto wrote:

#map_v accepted.

Matz.

Will all due respect,
I think the name #map_v is not very Ruby-like.
Why such a short name?

#map_values seems more consistent with existing methods (#values, #values_at, #each_value, #has_value?, #value?).
It also conveys the intention explicitly, while I am fairly certain many people will be confused by #map_v.
From the documentation and standard terminology: "A Hash is a dictionary-like collection of unique keys and their values.".
Most of the duplicated tickets also mentioned #map_values and I believe this is the name most people would expect.
The discussion in https://bugs.ruby-lang.org/issues/7793 also converges towards #map_values (even nobu agreed :) ).

Matz, could you please reconsider the name?

#16 [ruby-core:76821] Updated by rosenfeld (Rodrigo Rosenfeld Rosas) 10 months ago

Yes, I would say the same a few days ago about map_v being a bad name... I also prefer map_values. I just didn't say anything because I prefer a map_v over not having such a method at all and didn't want to delay this feature :P

#17 [ruby-core:76822] Updated by matz (Yukihiro Matsumoto) 10 months ago

Benoit,

The name and behavior haven't fixed yet. So we welcome discussion.

Your statement about "Ruby-like" is pretty interesting. Even though I have been designing Ruby for last 20+ years, I am not sure if I could define "Ruby-like". Could you please suggest a better "Ruby-like" name than "map_v"?

And Rodrigo, we have decided to introduce a method, so you don't have to worry about the delay.

Matz.

#18 [ruby-core:76826] Updated by marcandre (Marc-Andre Lafortune) 10 months ago

map_values is good, succinct and clear. 'v' is an unclear abbreviation, made worse by the existence of other methods with the long form like values_at.

I guess I'm just repeating what Jan and Rodrigo said...

#19 [ruby-core:76829] Updated by Eregon (Benoit Daloze) 10 months ago

Yukihiro Matsumoto wrote:

Benoit,

The name and behavior haven't fixed yet. So we welcome discussion.

Your statement about "Ruby-like" is pretty interesting. Even though I have been designing Ruby for last 20+ years, I am not sure if I could define "Ruby-like". Could you please suggest a better "Ruby-like" name than "map_v"?

Matz.

Dear Matz,

Thank you for the quick reply!
I think #map_values is intuitive and consistent with other methods as well as the general terminology used in Hash.
It seems almost everyone agrees on the name for once :)

About defining "Ruby-like" for names, that's though indeed.
I would think it's often concise yet clear and intuitive, generally avoiding abbreviations
and using well-established names from e.g. Unix and functional programming if there are.
They also have their own syntactic particularities with _, ?, ! and =.

#20 [ruby-core:76871] Updated by ko1 (Koichi Sasada) 9 months ago

  • Assignee changed from mrkn (Kenta Murata) to matz (Yukihiro Matsumoto)
  • Status changed from Closed to Open

reopened ticket because of naming issue.

Matz, could you close it if you finish the discussion?

#21 [ruby-core:76877] Updated by matz (Yukihiro Matsumoto) 9 months ago

  • Assignee changed from matz (Yukihiro Matsumoto) to mrkn (Kenta Murata)

From our discussion, we concerned map_values is too close to map.values, which has totally different semantics. Besides that, we are thinking about adding other methods like map_k and map_kv.

Matz.

#22 [ruby-core:76878] Updated by phluid61 (Matthew Kerwin) 9 months ago

Incidentally, as discussed back in #7793, the hashmap gem defines #map_keys, #map_values, and #map_pairs, and their bang variants.

#23 [ruby-core:76880] Updated by marcandre (Marc-Andre Lafortune) 9 months ago

Yukihiro Matsumoto wrote:

From our discussion, we concerned map_values is too close to map.values, which has totally different semantics.

But isn't map.values non sensical?

{}.map.values # => NoMethodError: undefined method `values' for #<Enumerator: {}:map>

Moreover, it usually will be map_values { |k,v| ... } (i.e. followed by a block), while we never see h.values { ... }.

I don't see how there could be confusion here.

#24 [ruby-core:76881] Updated by matz (Yukihiro Matsumoto) 9 months ago

  • Assignee changed from mrkn (Kenta Murata) to matz (Yukihiro Matsumoto)

map_values, map_keys and map_pairs seems consistent.

In addition, we are considering adding a method to create a hash from an enumerable in a similar way to map_pairs or map_kv. From my point of view, adding map_kv to Enumerable is acceptable, but map_pairs looks weird. You may feel differently, and if so, what should be the name of a method we are going to add to Enumerable?

Matz.

#25 [ruby-core:76923] Updated by Eregon (Benoit Daloze) 9 months ago

Yukihiro Matsumoto wrote:

map_values, map_keys and map_pairs seems consistent.

In addition, we are considering adding a method to create a hash from an enumerable in a similar way to map_pairs or map_kv.

What would be the semantics?
For simple cases there is already Enumerable#to_h for an enumeration of pairs:

(1..4).each_cons(2).map {|k,v| ... }.to_h

For more complex cases, would it be something like Enumerable#associate or #categorize (#4151) ?

#26 [ruby-core:76933] Updated by shyouhei (Shyouhei Urabe) 9 months ago

Can we stick to the name of #map_v here? I don't think it's wise to expand the front line in this thread.

#27 [ruby-core:76935] Updated by Eregon (Benoit Daloze) 9 months ago

Right, sorry for the diversion.

I agree with Marc-Andre, I do not find map_values confusing.
After all we want to take a Hash, change/map its values to something else and return the resulting Hash.

I guess matz meant hash.values.map {} but that's the reverse order of operations and naturally it ends up with a different result (take the Hash values as an Array and map it).
I do not see this as confusing.
Also there is already flat_map, collect_concat, etc which of course exist because it's not "just" a composition of the two operations mentioned in the name.

Enumerable#map_pairs sounds right if it's about manipulating pairs (2-element arrays or key-value pairs).

#28 [ruby-core:77173] Updated by matz (Yukihiro Matsumoto) 9 months ago

Ok, I will introduce transform_values (not map_v nor map_values).

I wanted a Hash generation method in Enumerable (e.g. map_kv), and the proposed method name to be consistent with the name.
But I found out that they are not really "map" functions, so they shouldn't be named map_*. So I gave up the idea of map_v and map_kv altogether. The original name transform_values describes the intention better.

Matz.

#29 Updated by mrkn (Kenta Murata) 9 months ago

  • Status changed from Open to Closed

Applied in changeset r56099.


hash.c: map_v -> transform_values

  • hash.c (rb_hash_transform_values, rb_hash_transform_values_bang):
    Rename map_v to transform_values.
    [Feature #12512]

  • test/ruby/test_hash.rb: ditto.

#30 Updated by shyouhei (Shyouhei Urabe) 7 months ago

Also available in: Atom PDF