Project

General

Profile

Actions

Feature #12512

closed

Import Hash#transform_values and its destructive version from ActiveSupport

Added by mrkn (Kenta Murata) over 7 years ago. Updated over 7 years ago.

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

Description

I think value transformation is a fundamental feature of Hash.


Related issues 6 (0 open6 closed)

Related to Ruby master - Feature #7793: New methods on HashClosedmatz (Yukihiro Matsumoto)Actions
Related to Ruby master - Feature #6669: A method like Hash#map but returns hashClosedmatz (Yukihiro Matsumoto)Actions
Related to Ruby master - Feature #8951: Please add a hash-to-hash alternative of the map method to HashClosedActions
Related to Ruby master - Feature #9635: Map a hash directly to a hashClosedActions
Related to Ruby master - Feature #9970: Add `Hash#map_keys` and `Hash#map_values`Closednobu (Nobuyoshi Nakada)Actions
Related to Ruby master - Feature #10208: Passing block to Enumerable#to_hClosedActions
Actions #1

Updated by mrkn (Kenta Murata) over 7 years ago

Updated by mrkn (Kenta Murata) over 7 years ago

  • Description updated (diff)

Updated by mrkn (Kenta Murata) over 7 years ago

  • Description updated (diff)
Actions #4

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

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

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

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

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

Actions #7

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

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

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

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

I believe no one is against the feature itself.

Updated by shyouhei (Shyouhei Urabe) over 7 years 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.

Updated by mrkn (Kenta Murata) over 7 years ago

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

Updated by matz (Yukihiro Matsumoto) over 7 years ago

#map_v accepted.

Matz.

Updated by mrkn (Kenta Murata) over 7 years ago

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

Updated by mrkn (Kenta Murata) over 7 years 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] [ruby-core:76095]

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

Updated by janfri (Jan Friedrich) over 7 years 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?

Updated by Eregon (Benoit Daloze) over 7 years 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?

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) over 7 years 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

Updated by matz (Yukihiro Matsumoto) over 7 years 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.

Updated by marcandre (Marc-Andre Lafortune) over 7 years 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...

Updated by Eregon (Benoit Daloze) over 7 years 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 =.

Updated by ko1 (Koichi Sasada) over 7 years ago

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

reopened ticket because of naming issue.

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

Updated by matz (Yukihiro Matsumoto) over 7 years 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.

Updated by phluid61 (Matthew Kerwin) over 7 years ago

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

Updated by marcandre (Marc-Andre Lafortune) over 7 years 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.

Updated by matz (Yukihiro Matsumoto) over 7 years 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.

Updated by Eregon (Benoit Daloze) over 7 years 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) ?

Updated by shyouhei (Shyouhei Urabe) over 7 years 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.

Updated by Eregon (Benoit Daloze) over 7 years 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).

Updated by matz (Yukihiro Matsumoto) over 7 years 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.

Actions #29

Updated by mrkn (Kenta Murata) over 7 years 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] [ruby-core:76095]

  • test/ruby/test_hash.rb: ditto.

Actions #30

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0