Feature #16252
openHash#partition should return hashes
Added by Dan0042 (Daniel DeLorme) almost 5 years ago. Updated over 2 years ago.
Description
Hash#partition is implemented by Enumerable so it just returns two arrays of arrays
{1=>2,3=>4}.partition{|k,|k==1} #=> [[[1, 2]], [[3, 4]]]
But I think it would make more sense to behave similarly to Hash#select and Hash#reject
{1=>2,3=>4}.partition{|k,|k==1} #=> [{1=>2}, {3=>4}]
Updated by Dan0042 (Daniel DeLorme) almost 3 years ago
Two years later, and today when using hash.partition I was surprised that it didn't return two hashes.
This has to be just an oversight, right?
Updated by zverok (Victor Shepelev) almost 3 years ago
I am afraid we don't have some particular rules for Enumerables to return their own class:
Set[*1..8].select(&:odd?)
# => [1, 3, 5, 7] -- not a Set
Making Hash#select
and #reject
return hashes was an ad-hoc decision (and quite useful at that!); maybe it would be good to consistently follow this decision for some core collections?
FWIF, I once made a gem to do that for any collection of your liking, and while working on it, I identified lists of methods where it is probably desirable to return same class: methods returning singular collection and methods returning several collections.
Updated by Dan0042 (Daniel DeLorme) almost 3 years ago
I think it's ok that we don't have some particular rules for Enumerables to return their own class. Rather than consistently follow a decision, I think it's good to evaluate each case on its own merits. That said, I can certainly imagine Set#select/reject/partition returning Sets. But that would have to be a separate proposal, maybe related to #16989.
Hash#reject has returned a Hash since at least 1.8. Making Hash#select return a Hash (in 1.9) has indeed been quite useful, and it would also be useful if #partition did the same. It's a bit surprising that hash.select(&b) != hash.partition(&b).first
Updated by Dan0042 (Daniel DeLorme) almost 3 years ago
FWIW, current usage in gems is consistent with Hash, but I did find one incompatibility in fugit-1.3.3.
actionpack-6.0.0/lib/action_dispatch/routing/mapper.rb
267: constraints.partition do |key, requirement|
subarrays then converted with Hash[subarray]
capybara-3.29.0/lib/capybara/selector/definition/element.rb
21: booleans, values = options.partition { |_k, v| [true, false].include? v }.map(&:to_h)
subarrays converted with .map(&:to_h)
fog-openstack-1.0.9/lib/fog/openstack/image/v1/models/images.rb
32: custom_properties, params = headers.partition do |k, _|
subarrays converted with .map { |p| Hash[p] }
fugit-1.3.3/lib/fugit/duration.rb
71: INFLA_KEYS, NON_INFLA_KEYS =
72: KEYS.partition { |k, v| v[:I] }
subarrays iterated with .each do |k, a|
However there is incompatibility with keys = INFLA_KEYS.dup; keys.unshift([ :mon, { s: Fugit::Duration.parse(mon).to_sec } ])
sequel-5.24.0/lib/sequel/plugins/auto_validations.rb
167: not_null_cols, explicit_not_null_cols = db_schema.select{|col, sch| sch[:allow_null] == false}.partition{|col, sch| sch[:default].nil?}.map{|cs| cs.map{|col, sch| col}}
subarrays converted to keys with .map{|cs| cs.map{|col, sch| col}}
would have same result with hash but could be written .map(&:keys)
instead
sprockets-3.7.2/lib/sprockets/cache/file_store.rb
167: delete_caches, keep_caches = caches.partition { |filename, stat|
subarrays compatible with hash: .map(&:first)
and .inject(0) { |sum, (_, stat)|
Updated by matz (Yukihiro Matsumoto) over 2 years ago
We worry about compatibility. It's too hard to estimate the damage. Maybe we need a new name?
Matz.
Updated by knu (Akinori MUSHA) over 2 years ago
I only wish partition
had been changed this way along with select
/reject
...
From my experience, partition
is likely used in business logic, so it is not safe to break the compatibility judging from public code search. In today's developer meeting, we discussed this and many pointed out that this functionality should have a new name, keeping partition
as it is now.
Updated by Dan0042 (Daniel DeLorme) over 2 years ago
knu (Akinori MUSHA) wrote in #note-6:
I only wish
partition
had been changed this way along withselect
/reject
...
If most people agree "it should have been this way" then why not bite the bullet and fix to the behavior that "should have been". Better late than never.
I agree it's hard to estimate the damage. Based on the code search above my best guess is "reasonable". But isn't that what release candidates are for? In the past matz has often said "let's try it" and revert if it causes too many problems, like freezing Symbol#to_s. I think this is one of those times.
I get the feeling that a new method like partition_h
would only be making this problem (mere annoyance?) worse.