From 1e979689398d92d507bb84f610bab25152a57d4e Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Tue, 23 Jan 2018 04:30:08 +0100 Subject: [PATCH 1/2] replace the API of replace --- base/set.jl | 89 +++++++++++++++++++++++++++++++++++---------------- test/sets.jl | 90 +++++++++++++++++++++++++--------------------------- 2 files changed, 104 insertions(+), 75 deletions(-) diff --git a/base/set.jl b/base/set.jl index d5948e075907d..138113e48074f 100644 --- a/base/set.jl +++ b/base/set.jl @@ -573,6 +573,8 @@ convert(::Type{T}, s::AbstractSet) where {T<:AbstractSet} = T(s) For each pair `old=>new` in `old_new`, replace all occurrences of `old` in collection `A` by `new`. +As a special case, if `new` is `nothing`, then the occurence is deleted; +wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. If `count` is specified, then replace at most `count` occurrences in total. See also [`replace`](@ref replace(A, old_new::Pair...)). @@ -587,17 +589,26 @@ julia> replace!([1, 2, 1, 3], 1=>0, 2=>4, count=2) julia> replace!(Set([1, 2, 3]), 1=>0) Set([0, 2, 3]) + +julia> replace!(Any[1, 2, 3], 1=>nothing, 3=>Some(nothing)) +[2, nothing] ``` """ replace!(A, old_new::Pair...; count::Integer=typemax(Int)) = _replace!(A, eltype(A), count, old_new) +wrap_nothing(x) = x +wrap_nothing(::Nothing) = Some(nothing) +unwrap_nothing(x) = x +unwrap_nothing(::Some{Nothing}) = nothing + # we use this wrapper because using directly eltype(A) as the type # parameter below for Some degrades performance function _replace!(A, ::Type{K}, count::Integer, old_new::Tuple{Vararg{Pair}}) where K @inline function prednew(x) for o_n in old_new - first(o_n) == x && return Some{K}(last(o_n)) + first(o_n) == x && return last(o_n) end + wrap_nothing(x) end replace!(prednew, A, count=count) end @@ -607,6 +618,8 @@ end Replace all occurrences `x` in collection `A` for which `pred(x)` is true by `new`. +As a special case, if `new` is `nothing`, then the occurence is deleted; +wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. # Examples ```jldoctest @@ -621,19 +634,19 @@ julia> replace!(isodd, A, 0, count=2) ``` """ replace!(pred::Callable, A, new; count::Integer=typemax(Int)) = - replace!(x -> if pred(x) Some(new) end, A, count=count) + replace!(x -> ifelse(pred(x), new, wrap_nothing(x)), A, count=count) """ - replace!(prednew::Function, A; [count::Integer]) + replace!(new::Function, A; [count::Integer]) -For each value `x` in `A`, `prednew(x)` is called and must -return either `nothing`, in which case no replacement occurs, -or a value, possibly wrapped as a [`Some`](@ref) object, which -will be used as a replacement for `x`. +Replace all occurrences `x` in collection `A` +by `new(x)`. +As a special case, if `new(x)` is `nothing`, then the occurence is deleted; +wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. # Examples ```jldoctest -julia> replace!(x -> isodd(x) ? 2x : nothing, [1, 2, 3, 4]) +julia> replace!(x -> isodd(x) ? 2x : x, [1, 2, 3, 4]) 4-element Array{Int64,1}: 2 2 @@ -641,7 +654,7 @@ julia> replace!(x -> isodd(x) ? 2x : nothing, [1, 2, 3, 4]) 4 julia> replace!(Union{Int,Nothing}[0, 1, 2, nothing, 4], count=2) do x - x !== nothing && iseven(x) ? Some(nothing) : nothing + x !== nothing && iseven(x) ? Some(nothing) : x end 5-element Array{Union{Nothing,Int64},1}: nothing @@ -651,7 +664,7 @@ julia> replace!(Union{Int,Nothing}[0, 1, 2, nothing, 4], count=2) do x 4 julia> replace!(Dict(1=>2, 3=>4)) do kv - if first(kv) < 3; first(kv)=>3 end + first(kv) < 3 ? first(kv)=>3 : kv end Dict{Int64,Int64} with 2 entries: 3 => 4 @@ -706,7 +719,7 @@ julia> replace(isodd, [1, 2, 3, 1], 0, count=2) ``` """ replace(pred::Callable, A, new; count::Integer=typemax(Int)) = - replace!(x -> if pred(x) Some(new) end, copy(A), count=count) + replace!(pred, copy(A), new, count=count) """ replace(prednew::Function, A; [count::Integer]) @@ -758,43 +771,63 @@ replace(a::AbstractString, b::Pair, c::Pair) = throw(MethodError(replace, (a, b, askey(k, ::AbstractDict) = k.first askey(k, ::AbstractSet) = k -function _replace_update_dict!(repl::Vector{<:Pair}, x, y::Some) - push!(repl, x => y.value) - true +function _replace_update_dict!(repl::Vector{<:Pair}, x, y) + if x === y + false + else + push!(repl, x => y) + true + end end -_replace_update_dict!(repl::Vector{<:Pair}, x, ::Nothing) = false -_replace_update_dict!(repl::Vector{<:Pair}, x, y) = _replace_update_dict!(repl, x, Some(y)) - function _replace!(prednew::Callable, A::Union{AbstractDict,AbstractSet}, count::Int) - repl = Pair{eltype(A),eltype(A)}[] + repl = Pair{eltype(A),eltype(A)}[] # first == second means delete c = 0 for x in A - c += _replace_update_dict!(repl, x, prednew(x)) + y = prednew(x) + if y === nothing + push!(repl, x=>x) # delete + c += 1 + else + c += _replace_update_dict!(repl, x, unwrap_nothing(y)) + end c == count && break end for oldnew in repl pop!(A, askey(first(oldnew), A)) end for oldnew in repl - push!(A, last(oldnew)) + first(oldnew) !== last(oldnew) && push!(A, last(oldnew)) end end ### AbstractArray -function _replace_update!(A::AbstractArray, i::Integer, y::Some) - @inbounds A[i] = y.value - true +function _replace_update!(A::AbstractArray, i::Integer, del::Int, y) + @inbounds begin + rep = A[i] !== y + if rep | (del != 0) + A[i-del] = y + end + return rep + end end -_replace_update!(A::AbstractArray, i::Integer, ::Nothing) = false -_replace_update!(A::AbstractArray, i::Integer, y) = _replace_update!(A, i, Some(y)) - function _replace!(prednew::Callable, A::AbstractArray, count::Int) c = 0 - for i in eachindex(A) - c += _replace_update!(A, i, prednew(A[i])) + del = 0 # number of deleted items + @inbounds for i in eachindex(A) + y = prednew(A[i]) + if y === nothing + A::AbstractVector + del += 1 + c += 1 + else + c += _replace_update!(A, i, del, unwrap_nothing(y)) + end c == count && break end + if del > 0 # implies A isa AbstractVector + resize!(A, length(A)-del) + end end diff --git a/test/sets.jl b/test/sets.jl index 6509177a82afb..2cafbc50a4602 100644 --- a/test/sets.jl +++ b/test/sets.jl @@ -492,47 +492,43 @@ end end @testset "replace! & replace" begin - maybe1(v, p) = if p Some(v) end - maybe2(v, p) = if p v end - - for maybe = (maybe1, maybe2) - a = [1, 2, 3, 1] - @test replace(x->maybe(2x, iseven(x)), a) == [1, 4, 3, 1] - @test replace!(x->maybe(2x, iseven(x)), a) === a - @test a == [1, 4, 3, 1] - @test replace(a, 1=>0) == [0, 4, 3, 0] - for count = (1, 0x1, big(1)) - @test replace(a, 1=>0, count=count) == [0, 4, 3, 1] - end - @test replace!(a, 1=>2) === a - @test a == [2, 4, 3, 2] - @test replace!(x->2x, a, count=0x2) == [4, 8, 3, 2] - - d = Dict(1=>2, 3=>4) - @test replace(x->x.first > 2, d, 0=>0) == Dict(1=>2, 0=>0) - @test replace!(x->maybe(x.first=>2*x.second, x.first > 2), d) === d - @test d == Dict(1=>2, 3=>8) - @test replace(d, (3=>8)=>(0=>0)) == Dict(1=>2, 0=>0) - @test replace!(d, (3=>8)=>(2=>2)) === d - @test d == Dict(1=>2, 2=>2) - for count = (1, 0x1, big(1)) - @test replace(x->x.second == 2, d, 0=>0, count=count) in [Dict(1=>2, 0=>0), - Dict(2=>2, 0=>0)] - end - s = Set([1, 2, 3]) - @test replace(x->maybe(2x, x>1), s) == Set([1, 4, 6]) - for count = (1, 0x1, big(1)) - @test replace(x->maybe(2x, x>1), s, count=count) in [Set([1, 4, 3]), Set([1, 2, 6])] - end - @test replace(s, 1=>4) == Set([2, 3, 4]) - @test replace!(s, 1=>2) === s - @test s == Set([2, 3]) - @test replace!(x->2x, s, count=0x1) in [Set([4, 3]), Set([2, 6])] + a = [1, 2, 3, 1] + @test replace(x -> iseven(x) ? 2x : x, a) == [1, 4, 3, 1] + @test replace!(x -> iseven(x) ? 2x : x, a) === a + @test a == [1, 4, 3, 1] + @test replace(a, 1=>0) == [0, 4, 3, 0] + for count = (1, 0x1, big(1)) + @test replace(a, 1=>0, count=count) == [0, 4, 3, 1] + end + @test replace!(a, 1=>2) === a + @test a == [2, 4, 3, 2] + @test replace!(x->2x, a, count=0x2) == [4, 8, 3, 2] + + d = Dict(1=>2, 3=>4) + @test replace(x->x.first > 2, d, 0=>0) == Dict(1=>2, 0=>0) + @test replace!(x -> x.first > 2 ? x.first=>2*x.second : x, d) === d + @test d == Dict(1=>2, 3=>8) + @test replace(d, (3=>8)=>(0=>0)) == Dict(1=>2, 0=>0) + @test replace!(d, (3=>8)=>(2=>2)) === d + @test d == Dict(1=>2, 2=>2) + for count = (1, 0x1, big(1)) + @test replace(x->x.second == 2, d, 0=>0, count=count) in [Dict(1=>2, 0=>0), + Dict(2=>2, 0=>0)] + end + s = Set([1, 2, 3]) + @test replace(x -> x > 1 ? 2x : x, s) == Set([1, 4, 6]) + for count = (1, 0x1, big(1)) + @test replace(x -> x > 1 ? 2x : x, s, count=count) in [Set([1, 4, 3]), Set([1, 2, 6])] + end + @test replace(s, 1=>4) == Set([2, 3, 4]) + @test replace!(s, 1=>2) === s + @test s == Set([2, 3]) + @test replace!(x->2x, s, count=0x1) in [Set([4, 3]), Set([2, 6])] - for count = (0, 0x0, big(0)) - @test replace([1, 2], 1=>0, 2=>0, count=count) == [1, 2] # count=0 --> no replacements - end + for count = (0, 0x0, big(0)) + @test replace([1, 2], 1=>0, 2=>0, count=count) == [1, 2] # count=0 --> no replacements end + # test collisions with AbstractSet/AbstractDict @test replace!(x->2x, Set([3, 6])) == Set([6, 12]) @test replace!(x->2x, Set([1:20;])) == Set([2:2:40;]) @@ -541,16 +537,16 @@ end # test Some(nothing) a = [1, 2, nothing, 4] @test replace(x -> x === nothing ? 0 : Some(nothing), a) == [nothing, nothing, 0, nothing] - @test replace(x -> x === nothing ? 0 : nothing, a) == [1, 2, 0, 4] - @test replace!(x -> x !== nothing ? Some(nothing) : nothing, a) == [nothing, nothing, nothing, nothing] - @test replace(iseven, Any[1, 2, 3, 4], nothing) == [1, nothing, 3, nothing] - @test replace(Any[1, 2, 3, 4], 1=>nothing, 3=>nothing) == [nothing, 2, nothing, 4] + @test replace(x -> x === nothing ? 0 : x, a) == [1, 2, 0, 4] + @test replace!(x -> x !== nothing ? Some(nothing) : Some(x), a) == [nothing, nothing, nothing, nothing] + @test replace(iseven, Any[1, 2, 3, 4], Some(nothing)) == [1, nothing, 3, nothing] + @test replace(Any[1, 2, 3, 4], 1=>Some(nothing), 3=>Some(nothing)) == [nothing, 2, nothing, 4] s = Set([1, 2, nothing, 4]) @test replace(x -> x === nothing ? 0 : Some(nothing), s) == Set([0, nothing]) - @test replace(x -> x === nothing ? 0 : nothing, s) == Set([1, 2, 0, 4]) - @test replace(x -> x !== nothing ? Some(nothing) : nothing, s) == Set([nothing]) - @test replace(iseven, Set(Any[1, 2, 3, 4]), nothing) == Set([1, nothing, 3, nothing]) - @test replace(Set(Any[1, 2, 3, 4]), 1=>nothing, 3=>nothing) == Set([nothing, 2, nothing, 4]) + @test replace(x -> x === nothing ? 0 : x, s) == Set([1, 2, 0, 4]) + @test replace(x -> x !== nothing ? Some(nothing) : x, s) == Set([nothing]) + @test replace(iseven, Set(Any[1, 2, 3, 4]), Some(nothing)) == Set([1, nothing, 3, nothing]) + @test replace(Set(Any[1, 2, 3, 4]), 1=>Some(nothing), 3=>Some(nothing)) == Set([nothing, 2, nothing, 4]) # avoid recursive call issue #25384 @test_throws MethodError replace!("") From f19586ecefa9416b549e5c00dd3277339ba7606f Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Thu, 25 Jan 2018 11:46:59 +0100 Subject: [PATCH 2/2] make nothing not special --- base/set.jl | 119 ++++++++++++--------------------------------------- test/sets.jl | 15 ++----- 2 files changed, 30 insertions(+), 104 deletions(-) diff --git a/base/set.jl b/base/set.jl index 138113e48074f..9af76ec4ba4f6 100644 --- a/base/set.jl +++ b/base/set.jl @@ -565,7 +565,7 @@ convert(::Type{T}, s::AbstractSet) where {T<:AbstractSet} = T(s) ## replace/replace! ## # to make replace/replace! work for a new container type Cont, only -# replace!(prednew::Callable, A::Cont; count::Integer=typemax(Int)) +# replace!(new::Callable, A::Cont; count::Integer=typemax(Int)) # has to be implemented """ @@ -573,8 +573,6 @@ convert(::Type{T}, s::AbstractSet) where {T<:AbstractSet} = T(s) For each pair `old=>new` in `old_new`, replace all occurrences of `old` in collection `A` by `new`. -As a special case, if `new` is `nothing`, then the occurence is deleted; -wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. If `count` is specified, then replace at most `count` occurrences in total. See also [`replace`](@ref replace(A, old_new::Pair...)). @@ -589,28 +587,18 @@ julia> replace!([1, 2, 1, 3], 1=>0, 2=>4, count=2) julia> replace!(Set([1, 2, 3]), 1=>0) Set([0, 2, 3]) - -julia> replace!(Any[1, 2, 3], 1=>nothing, 3=>Some(nothing)) -[2, nothing] ``` """ replace!(A, old_new::Pair...; count::Integer=typemax(Int)) = _replace!(A, eltype(A), count, old_new) -wrap_nothing(x) = x -wrap_nothing(::Nothing) = Some(nothing) -unwrap_nothing(x) = x -unwrap_nothing(::Some{Nothing}) = nothing - -# we use this wrapper because using directly eltype(A) as the type -# parameter below for Some degrades performance function _replace!(A, ::Type{K}, count::Integer, old_new::Tuple{Vararg{Pair}}) where K - @inline function prednew(x) + @inline function new(x) for o_n in old_new first(o_n) == x && return last(o_n) end - wrap_nothing(x) + return x # no replace end - replace!(prednew, A, count=count) + replace!(new, A, count=count) end """ @@ -618,8 +606,6 @@ end Replace all occurrences `x` in collection `A` for which `pred(x)` is true by `new`. -As a special case, if `new` is `nothing`, then the occurence is deleted; -wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. # Examples ```jldoctest @@ -634,15 +620,14 @@ julia> replace!(isodd, A, 0, count=2) ``` """ replace!(pred::Callable, A, new; count::Integer=typemax(Int)) = - replace!(x -> ifelse(pred(x), new, wrap_nothing(x)), A, count=count) + replace!(x -> ifelse(pred(x), new, x), A, count=count) """ replace!(new::Function, A; [count::Integer]) -Replace all occurrences `x` in collection `A` -by `new(x)`. -As a special case, if `new(x)` is `nothing`, then the occurence is deleted; -wrap `nothing` as `Some(nothing)` if `nothing` must be used as the new value. +Replace each element `x` in collection `A` by `new(x)`. +If `count` is specified, then replace at most `count` values in total +(replacements being defined as `new(x) !== x`). # Examples ```jldoctest @@ -653,16 +638,6 @@ julia> replace!(x -> isodd(x) ? 2x : x, [1, 2, 3, 4]) 6 4 -julia> replace!(Union{Int,Nothing}[0, 1, 2, nothing, 4], count=2) do x - x !== nothing && iseven(x) ? Some(nothing) : x - end -5-element Array{Union{Nothing,Int64},1}: - nothing - 1 - nothing - nothing - 4 - julia> replace!(Dict(1=>2, 3=>4)) do kv first(kv) < 3 ? first(kv)=>3 : kv end @@ -674,10 +649,10 @@ julia> replace!(x->2x, Set([3, 6])) Set([6, 12]) ``` """ -function replace!(prednew::Callable, A::Union{AbstractArray,AbstractDict,AbstractSet}; +function replace!(new::Callable, A::Union{AbstractArray,AbstractDict,AbstractSet}; count::Integer=typemax(Int)) count < 0 && throw(DomainError(count, "`count` must not be negative")) - count != 0 && _replace!(prednew, A, min(count, typemax(Int)) % Int) + count != 0 && _replace!(new, A, min(count, typemax(Int)) % Int) A end @@ -722,41 +697,28 @@ replace(pred::Callable, A, new; count::Integer=typemax(Int)) = replace!(pred, copy(A), new, count=count) """ - replace(prednew::Function, A; [count::Integer]) + replace(new::Function, A; [count::Integer]) -Return a copy of `A` where for each value `x` in `A`, `prednew(x)` is called -and must return either `nothing`, in which case no replacement occurs, -or a value, possibly wrapped as a [`Some`](@ref) object, which -will be used as a replacement for `x`. +Return a copy of `A` where each value `x` in `A` is replaced by `new(x)` # Examples ```jldoctest -julia> replace(x -> isodd(x) ? 2x : nothing, [1, 2, 3, 4]) +julia> replace(x -> isodd(x) ? 2x : x, [1, 2, 3, 4]) 4-element Array{Int64,1}: 2 2 6 4 -julia> replace(Union{Int,Nothing}[0, 1, 2, nothing, 4], count=2) do x - x !== nothing && iseven(x) ? Some(nothing) : nothing - end -5-element Array{Union{Nothing,Int64},1}: - nothing - 1 - nothing - nothing - 4 - julia> replace(Dict(1=>2, 3=>4)) do kv - if first(kv) < 3; first(kv)=>3 end + first(kv) < 3 ? first(kv)=>3 : kv end Dict{Int64,Int64} with 2 entries: 3 => 4 1 => 3 ``` """ -replace(prednew::Callable, A; count::Integer=typemax(Int)) = replace!(prednew, copy(A), count=count) +replace(new::Callable, A; count::Integer=typemax(Int)) = replace!(new, copy(A), count=count) # Handle ambiguities replace!(a::Callable, b::Pair; count::Integer=-1) = throw(MethodError(replace!, (a, b))) @@ -771,25 +733,14 @@ replace(a::AbstractString, b::Pair, c::Pair) = throw(MethodError(replace, (a, b, askey(k, ::AbstractDict) = k.first askey(k, ::AbstractSet) = k -function _replace_update_dict!(repl::Vector{<:Pair}, x, y) - if x === y - false - else - push!(repl, x => y) - true - end -end - -function _replace!(prednew::Callable, A::Union{AbstractDict,AbstractSet}, count::Int) - repl = Pair{eltype(A),eltype(A)}[] # first == second means delete +function _replace!(new::Callable, A::Union{AbstractDict,AbstractSet}, count::Int) + repl = Pair{eltype(A),eltype(A)}[] c = 0 for x in A - y = prednew(x) - if y === nothing - push!(repl, x=>x) # delete + y = new(x) + if x !== y + push!(repl, x => y) c += 1 - else - c += _replace_update_dict!(repl, x, unwrap_nothing(y)) end c == count && break end @@ -797,37 +748,21 @@ function _replace!(prednew::Callable, A::Union{AbstractDict,AbstractSet}, count: pop!(A, askey(first(oldnew), A)) end for oldnew in repl - first(oldnew) !== last(oldnew) && push!(A, last(oldnew)) + push!(A, last(oldnew)) end end ### AbstractArray -function _replace_update!(A::AbstractArray, i::Integer, del::Int, y) - @inbounds begin - rep = A[i] !== y - if rep | (del != 0) - A[i-del] = y - end - return rep - end -end - -function _replace!(prednew::Callable, A::AbstractArray, count::Int) +function _replace!(new::Callable, A::AbstractArray, count::Int) c = 0 - del = 0 # number of deleted items - @inbounds for i in eachindex(A) - y = prednew(A[i]) - if y === nothing - A::AbstractVector - del += 1 + for i in eachindex(A) + @inbounds Ai = A[i] + y = new(Ai) + if Ai !== y + @inbounds A[i] = y c += 1 - else - c += _replace_update!(A, i, del, unwrap_nothing(y)) end c == count && break end - if del > 0 # implies A isa AbstractVector - resize!(A, length(A)-del) - end end diff --git a/test/sets.jl b/test/sets.jl index 2cafbc50a4602..4c09c72b07026 100644 --- a/test/sets.jl +++ b/test/sets.jl @@ -534,19 +534,10 @@ end @test replace!(x->2x, Set([1:20;])) == Set([2:2:40;]) @test replace!(kv -> (2kv[1] => kv[2]), Dict(1=>2, 2=>4, 4=>8, 8=>16)) == Dict(2=>2, 4=>4, 8=>8, 16=>16) - # test Some(nothing) + # test nothing & Some(nothing) (should behave as any other value) a = [1, 2, nothing, 4] - @test replace(x -> x === nothing ? 0 : Some(nothing), a) == [nothing, nothing, 0, nothing] - @test replace(x -> x === nothing ? 0 : x, a) == [1, 2, 0, 4] - @test replace!(x -> x !== nothing ? Some(nothing) : Some(x), a) == [nothing, nothing, nothing, nothing] - @test replace(iseven, Any[1, 2, 3, 4], Some(nothing)) == [1, nothing, 3, nothing] - @test replace(Any[1, 2, 3, 4], 1=>Some(nothing), 3=>Some(nothing)) == [nothing, 2, nothing, 4] - s = Set([1, 2, nothing, 4]) - @test replace(x -> x === nothing ? 0 : Some(nothing), s) == Set([0, nothing]) - @test replace(x -> x === nothing ? 0 : x, s) == Set([1, 2, 0, 4]) - @test replace(x -> x !== nothing ? Some(nothing) : x, s) == Set([nothing]) - @test replace(iseven, Set(Any[1, 2, 3, 4]), Some(nothing)) == Set([1, nothing, 3, nothing]) - @test replace(Set(Any[1, 2, 3, 4]), 1=>Some(nothing), 3=>Some(nothing)) == Set([nothing, 2, nothing, 4]) + @test replace(x -> x === nothing ? Some(nothing) : nothing, a) == [nothing, nothing, Some(nothing), nothing] + @test replace(x -> x === nothing ? Some(nothing) : nothing, Set(a)) == Set([Some(nothing), nothing]) # avoid recursive call issue #25384 @test_throws MethodError replace!("")