Skip to content

Commit cd33115

Browse files
authored
fix replace for dicts/sets, when the eltype changes (#38267)
There were two bugs: 1. for dicts, `empty(d, elemtype)` was used instead of `empty(d, keytype, valtype)` 2. the lower-level `_replace!` routines were accepting only an old/new container pair with the exact same `eltype`, new items had to have the same type as old one.
1 parent d6a97a1 commit cd33115

File tree

2 files changed

+27
-5
lines changed

2 files changed

+27
-5
lines changed

base/set.jl

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ end
433433
# TODO: use copy!, which is currently unavailable from here since it is defined in Future
434434
_copy_oftype(x, ::Type{T}) where {T} = copyto!(similar(x, T), x)
435435
# TODO: use similar() once deprecation is removed and it preserves keys
436-
_copy_oftype(x::AbstractDict, ::Type{T}) where {T} = merge!(empty(x, T), x)
436+
_copy_oftype(x::AbstractDict, ::Type{Pair{K,V}}) where {K,V} = merge!(empty(x, K, V), x)
437437
_copy_oftype(x::AbstractSet, ::Type{T}) where {T} = union!(empty(x, T), x)
438438

439439
_copy_oftype(x::AbstractArray{T}, ::Type{T}) where {T} = copy(x)
@@ -617,8 +617,10 @@ replace(a::AbstractString, b::Pair, c::Pair) = throw(MethodError(replace, (a, b,
617617
askey(k, ::AbstractDict) = k.first
618618
askey(k, ::AbstractSet) = k
619619

620-
function _replace!(new::Callable, res::T, A::T,
621-
count::Int) where T<:Union{AbstractDict,AbstractSet}
620+
function _replace!(new::Callable, res::Union{AbstractDict,AbstractSet},
621+
A::Union{AbstractDict,AbstractSet}, count::Int)
622+
@assert res isa AbstractDict && A isa AbstractDict ||
623+
res isa AbstractSet && A isa AbstractSet
622624
count == 0 && return res
623625
c = 0
624626
if res === A # cannot replace elements while iterating over A
@@ -686,7 +688,7 @@ end
686688

687689
### specialization for Dict / Set
688690

689-
function _replace!(new::Callable, t::Dict{K,V}, A::Dict{K,V}, count::Int) where {K,V}
691+
function _replace!(new::Callable, t::Dict{K,V}, A::AbstractDict, count::Int) where {K,V}
690692
# we ignore A, which is supposed to be equal to the destination t,
691693
# as it can generally be faster to just replace inline
692694
count == 0 && return t
@@ -718,7 +720,7 @@ function _replace!(new::Callable, t::Dict{K,V}, A::Dict{K,V}, count::Int) where
718720
t
719721
end
720722

721-
function _replace!(new::Callable, t::Set{T}, ::Set{T}, count::Int) where {T}
723+
function _replace!(new::Callable, t::Set{T}, ::AbstractSet, count::Int) where {T}
722724
_replace!(t.dict, t.dict, count) do kv
723725
k = first(kv)
724726
k2 = new(k)

test/sets.jl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,26 @@ end
646646
x = @inferred replace([1, missing], missing=>2, 1=>missing)
647647
@test isequal(x, [missing, 2]) && x isa Vector{Union{Int, Missing}}
648648

649+
# eltype promotion for dicts
650+
d = Dict(1=>2, 3=>4)
651+
f = replace(d, (1=>2) => (1=>nothing))
652+
@test f == Dict(3=>4, 1=>nothing)
653+
@test eltype(f) == Pair{Int, Union{Nothing, Int}}
654+
f = replace(d, (1=>2) => (1=>missing), (3=>4)=>(3=>missing))
655+
@test valtype(f) == Union{Missing,Int}
656+
f = replace(d, (1=>2) => (1=>'a'), (3=>4)=>(3=>'b'))
657+
@test valtype(f) == Any
658+
@test f == Dict(3=>'b', 1=>'a')
659+
660+
# eltype promotion for sets
661+
s = Set([1, 2, 3])
662+
f = replace(s, 2=>missing, 3=>nothing)
663+
@test f == Set([1, missing, nothing])
664+
@test eltype(f) == Union{Int,Missing,Nothing}
665+
f = replace(s, 2=>'a')
666+
@test eltype(f) == Any
667+
@test f == Set([1, 3, 'a'])
668+
649669
# test that isequal is used
650670
@test replace([NaN, 1.0], NaN=>0.0) == [0.0, 1.0]
651671
@test replace([1, missing], missing=>0) == [1, 0]

0 commit comments

Comments
 (0)