Project

General

Profile

Actions

Feature #16252

open

Hash#partition should return hashes

Added by Dan0042 (Daniel DeLorme) over 2 years ago. Updated 6 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:95336]

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) 7 months 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) 7 months 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) 7 months 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) 7 months 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) 6 months 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) 6 months 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) 6 months ago

knu (Akinori MUSHA) wrote in #note-6:

I only wish partition had been changed this way along with select/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.

Actions

Also available in: Atom PDF