Bug #17757
closedHash#slice does not keep compare_by_identity on the results
Description
GH-PR: https://github.com/ruby/ruby/pull/4330
$ ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]
str1 = +'str'
str2 = +'str'
hash = {a: :a, b: :b, c: :c}.compare_by_identity
hash[str1] = 1
hash[str2] = 2
p hash.values_at(str1, str2) #=> [1, 2]
p hash.except.compare_by_identity? #=> true
p hash.slice.compare_by_identity? #=> false
p hash.except(str1, str2) #=> {:a=>:a, :b=>:b, :c=>:c}
p hash.slice(str1, str2) #=> {"str"=>2}
Is this an intentional behavior?
I would expect Hash#slice keeps compare_by_identity behaviors.
Updated by marcandre (Marc-Andre Lafortune) over 3 years ago
I should have looked into this when I checked #16996.
Looks like we have behavior that is all over the place, and the same method sometimes returns a hash with different comparability depending if the receiver is empty or not...
ruby 3.1.0dev (2021-03-28T18:18:08Z master 1cdecb4349) [x86_64-darwin18]
slice with arg: false
slice without arg: false
except with arg: true but false for {}
except without arg: true but false for {}
dup: true
select: true but false for {} # in 3.0: false
reject: true but false for {} # in 3.0: false
merge: true but false for {}
transform_values: true but false for {}
transform_keys: false
to_h: false
to_h without block: true
Script:
def check(method, label = method, *args, &block)
ci1, ci2 = [{a: 1}, {}].map do |h|
h.compare_by_identity
h2 = h.send(method, &block)
h2.compare_by_identity?
end
s = "#{label}: #{ci1}"
s << " but #{ci2} for {}" if ci2 != ci1
puts s
end
check(:slice, 'slice with arg', :a)
check(:slice, 'slice without arg')
check(:except, 'except with arg', :a)
check(:except, 'except without arg')
%i[dup select reject merge transform_values transform_keys to_h].each do |method|
check(method) { |k, v| [k, v] }
end
check(:to_h, 'to_h without block')
It's probably clear that the result should always be consistent for empty hash or not.
I feel the flag should be conserved for all these examples, except to_h {...}
which should create an array without comparison by identity (as it does currently).
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
I've submitted a pull request that builds on @kachick's PR and fixes the remaining cases found by @marcandre (Marc-Andre Lafortune): https://github.com/ruby/ruby/pull/4616
Updated by marcandre (Marc-Andre Lafortune) over 3 years ago
Thanks for the PR @jeremyevans0 (Jeremy Evans), looks pretty good.
On the other hand, I'd like to revise what I wrote and suggest that transform_keys
remain untouched. It's the only one of these methods (with to_h{...}
) that changes the keys so has a reason to interpret them differently.
Matz, do you agree?
Updated by matz (Yukihiro Matsumoto) over 3 years ago
Yes, we must keep compare_by_identity
status for those methods. Accepted.
Matz.
Updated by jeremyevans (Jeremy Evans) over 3 years ago
- Status changed from Open to Closed
Applied in changeset git|95f8ffa5f6c70aa9383e1f6db02b22707c183402.
Copy hash compare_by_identity setting in more cases
This makes the compare_by_identity setting always copied
for the following methods:
- except
- merge
- reject
- select
- slice
- transform_values
Some of these methods did not copy the setting, or only
copied the setting if the receiver was not empty.
Fixes [Bug #17757]
Co-authored-by: Kenichi Kamiya kachick1@gmail.com