From b64dc59075b610a72bdbcb51267dfa4f4bf7c715 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Mon, 7 Oct 2019 20:56:55 -0700 Subject: [PATCH 1/5] Implement setindex for AbstractArray and Dict --- base/array.jl | 37 +++++++++++++++++++++++++++++++++++++ base/dict.jl | 9 +++++++++ test/arrayops.jl | 8 ++++++++ test/dict.jl | 12 ++++++++++++ 4 files changed, 66 insertions(+) diff --git a/base/array.jl b/base/array.jl index 3449d7bc49a2d..15606f143e9dd 100644 --- a/base/array.jl +++ b/base/array.jl @@ -834,6 +834,43 @@ function setindex!(A::Array{T}, X::Array{T}, c::Colon) where T return A end +""" + setindex(collection, value, key...) + +Create a new collection with the element/elements of the location specified by `key` +replaced with `value`(s). + +# Implementation + +`setindex(collection, value, key...)` must have the property such that + +```julia +y1 = setindex(x, value, key...) + +y2 = deepcopy(x) +setindex!(y2, value, key...) + +@assert isequal′(y1, y2) +``` + +with a suitable definition of `isequal′` (e.g., elements are approximately but possibly +not exactly equal), _if_ `setindex!` supports given arguments. + +`setindex` should support more combinations of arguments by widening collection +type as required. +""" +function setindex end + +function setindex(xs::AbstractArray, v, I...) + T = promote_type(eltype(xs), typeof(v)) + ys = similar(xs, T) + if eltype(xs) !== Union{} + copy!(ys, xs) + end + ys[I...] = v + return ys +end + # efficiently grow an array _growbeg!(a::Vector, delta::Integer) = diff --git a/base/dict.jl b/base/dict.jl index 4cff4ee095fe9..ee760a7e8f6a7 100644 --- a/base/dict.jl +++ b/base/dict.jl @@ -391,6 +391,15 @@ function setindex!(h::Dict{K,V}, v0, key::K) where V where K return h end +function setindex(d0::Dict, v, k) + K = promote_type(keytype(d0), typeof(k)) + V = promote_type(valtype(d0), typeof(v)) + d = Dict{K, V}() + copy!(d, d0) + d[k] = v + return d +end + """ get!(collection, key, default) diff --git a/test/arrayops.jl b/test/arrayops.jl index 7a2fa864f543c..919e3e2245912 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -2650,3 +2650,11 @@ end # Throws ArgumentError for negative dimensions in Array @test_throws ArgumentError fill('a', -10) + +@testset "setindex" begin + ==ₜ(_, _) = false + ==ₜ(x::T, y::T) where T = x == y + + @test Base.setindex(Int[1, 2], 3.0, 2) ==ₜ [1.0, 3.0] + @test Base.setindex(Int[1, 2, 3], :two, 2) ==ₜ [1, :two, 3] +end diff --git a/test/dict.jl b/test/dict.jl index 1224d41bb220a..482213359c07b 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -1072,3 +1072,15 @@ end @test testdict[:b] == 1 end end + +@testset "setindex" begin + ==ₜ(_, _) = false + ==ₜ(x::T, y::T) where T = x == y + + @test Base.setindex(Dict(:a=>1, :b=>2), 10, :a) ==ₜ + Dict(:a=>10, :b=>2) + @test Base.setindex(Dict(:a=>1, :b=>2), 3, "c") ==ₜ + Dict(:a=>1, :b=>2, "c"=>3) + @test Base.setindex(Dict(:a=>1, :b=>2), 3.0, :c) ==ₜ + Dict(:a=>1.0, :b=>2.0, :c=>3.0) +end From 4743df1338335e08a94b67781847eb3f994b3947 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 10 Dec 2019 14:49:06 -0800 Subject: [PATCH 2/5] Improve setindex specification --- base/array.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/array.jl b/base/array.jl index 4fa9f025d5f77..726839f7dc846 100644 --- a/base/array.jl +++ b/base/array.jl @@ -847,14 +847,14 @@ replaced with `value`(s). ```julia y1 = setindex(x, value, key...) -y2 = deepcopy(x) -setindex!(y2, value, key...) +y2 = copy′(x) +@assert convert.(eltype(y2), x) == y2 -@assert isequal′(y1, y2) +y2[key...] = value +@assert convert.(eltype(y2), y1) == y2 ``` -with a suitable definition of `isequal′` (e.g., elements are approximately but possibly -not exactly equal), _if_ `setindex!` supports given arguments. +with a suitable definition of `copy′` such that `y2[key...] = value` succeeds. `setindex` should support more combinations of arguments by widening collection type as required. From 9f9065a03d6b65cd4ac31b4bbe39d87af8248306 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 10 Dec 2019 14:52:18 -0800 Subject: [PATCH 3/5] Remove special handling of `eltype(xs) !== Union{}` --- base/array.jl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/base/array.jl b/base/array.jl index 726839f7dc846..de9341cbd1e0c 100644 --- a/base/array.jl +++ b/base/array.jl @@ -864,9 +864,7 @@ function setindex end function setindex(xs::AbstractArray, v, I...) T = promote_type(eltype(xs), typeof(v)) ys = similar(xs, T) - if eltype(xs) !== Union{} - copy!(ys, xs) - end + copy!(ys, xs) ys[I...] = v return ys end From b204b6c16f435ae7f5ba0f848a0e863acf9c3f4c Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 11 Dec 2019 01:51:45 -0800 Subject: [PATCH 4/5] Test that setindex does not mutate input --- test/arrayops.jl | 6 ++++++ test/dict.jl | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/test/arrayops.jl b/test/arrayops.jl index 54ca33dcb0582..5680416ab4d8f 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -2657,6 +2657,12 @@ end @test Base.setindex(Int[1, 2], 3.0, 2) ==ₜ [1.0, 3.0] @test Base.setindex(Int[1, 2, 3], :two, 2) ==ₜ [1, :two, 3] + + @testset "no mutation" begin + arr = [1] + @test Base.setindex(arr, 2, 1) ==ₜ [2] + @test arr == [1] + end end @testset "Issue 33919" begin diff --git a/test/dict.jl b/test/dict.jl index 482213359c07b..7d6370798774d 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -1083,4 +1083,10 @@ end Dict(:a=>1, :b=>2, "c"=>3) @test Base.setindex(Dict(:a=>1, :b=>2), 3.0, :c) ==ₜ Dict(:a=>1.0, :b=>2.0, :c=>3.0) + + @testset "no mutation" begin + dict = Dict(:a=>1, :b=>2) + @test Base.setindex(dict, 10, :a) ==ₜ Dict(:a=>10, :b=>2) + @test dict == Dict(:a=>1, :b=>2) + end end From b57088b54a0a27e584dcce37591a5fdd66cb06b4 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 11 Dec 2019 01:54:09 -0800 Subject: [PATCH 5/5] Test inferrability of setindex --- test/arrayops.jl | 4 ++-- test/dict.jl | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index 5680416ab4d8f..d969a3d4e0db2 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -2655,8 +2655,8 @@ end ==ₜ(_, _) = false ==ₜ(x::T, y::T) where T = x == y - @test Base.setindex(Int[1, 2], 3.0, 2) ==ₜ [1.0, 3.0] - @test Base.setindex(Int[1, 2, 3], :two, 2) ==ₜ [1, :two, 3] + @test @inferred(Base.setindex(Int[1, 2], 3.0, 2)) ==ₜ [1.0, 3.0] + @test @inferred(Base.setindex(Int[1, 2, 3], :two, 2)) ==ₜ [1, :two, 3] @testset "no mutation" begin arr = [1] diff --git a/test/dict.jl b/test/dict.jl index 7d6370798774d..dbf36a6c4498b 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -1077,11 +1077,11 @@ end ==ₜ(_, _) = false ==ₜ(x::T, y::T) where T = x == y - @test Base.setindex(Dict(:a=>1, :b=>2), 10, :a) ==ₜ + @test @inferred(Base.setindex(Dict(:a=>1, :b=>2), 10, :a)) ==ₜ Dict(:a=>10, :b=>2) - @test Base.setindex(Dict(:a=>1, :b=>2), 3, "c") ==ₜ + @test @inferred(Base.setindex(Dict(:a=>1, :b=>2), 3, "c")) ==ₜ Dict(:a=>1, :b=>2, "c"=>3) - @test Base.setindex(Dict(:a=>1, :b=>2), 3.0, :c) ==ₜ + @test @inferred(Base.setindex(Dict(:a=>1, :b=>2), 3.0, :c)) ==ₜ Dict(:a=>1.0, :b=>2.0, :c=>3.0) @testset "no mutation" begin