Skip to content

Commit

Permalink
Bugfix setindex (#759)
Browse files Browse the repository at this point in the history
* bugfix setindex

* fix ambiguity
  • Loading branch information
rafaqz authored Aug 4, 2024
1 parent 2012cde commit d49dfc4
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 20 deletions.
8 changes: 5 additions & 3 deletions src/array/indexing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ for f in (:getindex, :view, :dotview)
@propagate_inbounds function $_dim_f(
A::AbstractBasicDimArray, dims::Union{Dimension,DimensionIndsArrays}...
)
return merge_and_index($f, A, dims)
return merge_and_index(Base.$f, A, dims)
end
end
# Standard indices
Expand Down Expand Up @@ -252,10 +252,12 @@ Base.@assume_effects :foldable @inline _simplify_dim_indices() = ()
#### setindex ####

@propagate_inbounds Base.setindex!(A::AbstractDimArray, x) = setindex!(parent(A), x)
@propagate_inbounds Base.setindex!(A::AbstractDimArray, x, args::Dimension...; kw...) =
setindex!(A, x, dims2indices(A, (args..., kw2dims(values(kw))...))...)
@propagate_inbounds Base.setindex!(A::AbstractDimArray, x, i, I...) =
setindex!(A, x, dims2indices(A, (i, I...))...)
@propagate_inbounds Base.setindex!(A::AbstractDimArray, x, I::DimensionalIndices...; kw...) =
setindex!(A, x, dims2indices(A, _simplify_dim_indices(I..., kw2dims(values(kw))...))...)
@propagate_inbounds Base.setindex!(::DimensionalData.AbstractDimArray, x, ::_DimIndicesAmb, ::_DimIndicesAmb...; kw...) =
setindex!(A, x, dims2indices(A, _simplify_dim_indices(I..., kw2dims(values(kw))...))...)
@propagate_inbounds Base.setindex!(A::AbstractDimArray, x, i1::StandardIndices, I::StandardIndices...) =
setindex!(parent(A), x, i1, I...)

Expand Down
16 changes: 5 additions & 11 deletions src/stack/indexing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -149,29 +149,23 @@ end
@propagate_inbounds Base.setindex!(s::AbstractDimStack, xs, I...; kw...) =
map((A, x) -> setindex!(A, x, I...; kw...), layers(s), xs)
@propagate_inbounds Base.setindex!(s::AbstractDimStack, xs::NamedTuple, i::Integer; kw...) =
hassamedims(s) ? _map_setindex!(s, xs, i) : _setindex_mixed(s, xs, i)
hassamedims(s) ? _map_setindex!(s, xs, i; kw...) : _setindex_mixed!(s, xs, i; kw...)
@propagate_inbounds Base.setindex!(s::AbstractDimStack, xs::NamedTuple, i::Colon; kw...) =
hassamedims(s) ? _map_setindex!(s, xs, i) : _setindex_mixed(s, xs, i)
hassamedims(s) ? _map_setindex!(s, xs, i; kw...) : _setindex_mixed!(s, xs, i; kw...)
@propagate_inbounds Base.setindex!(s::AbstractDimStack, xs::NamedTuple, i::AbstractArray; kw...) =
hassamedims(s) ? _map_setindex!(s, xs, i) : _setindex_mixed(s, xs, i)
hassamedims(s) ? _map_setindex!(s, xs, i; kw...) : _setindex_mixed!(s, xs, i; kw...)

@propagate_inbounds function Base.setindex!(
s::AbstractDimStack, xs::NamedTuple, I...; kw...
)
map((A, x) -> setindex!(A, x, I...; kw...), layers(s), xs)
end
# For ambiguity
# @propagate_inbounds function Base.setindex!(
# s::AbstractDimStack, xs::NamedTuple, i::Integer
# )
# setindex!(A, xs, DimIndices(s)[i])
# end

_map_setindex!(s, xs, i) = map((A, x) -> setindex!(A, x, i...; kw...), layers(s), xs)

_setindex_mixed(s::AbstractDimStack, x, i::AbstractArray) =
_setindex_mixed!(s::AbstractDimStack, x, i::AbstractArray) =
map(A -> setindex!(A, x, DimIndices(dims(s))[i]), layers(s))
_setindex_mixed(s::AbstractDimStack, i::Integer) =
_setindex_mixed!(s::AbstractDimStack, i::Integer) =
map(A -> setindex!(A, x, DimIndices(dims(s))[i]), layers(s))
function _setindex_mixed!(s::AbstractDimStack, x, i::Colon)
map(DimIndices(dims(s))) do D
Expand Down
20 changes: 20 additions & 0 deletions test/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,26 @@ end
@test @inferred(z .+ x) === 6
end

@testset "DimIndices broadcasting" begin
ds = X(1.0:0.2:2.0), Y(10:2:20)
A = rand(ds)
B = zeros(ds)
C = zeros(ds)
B[DimIndices(B)] .+= A
C[DimSelectors(C)] .+= A
@test A == B == C
sub = A[1:4, 1:3]
B .= 0
C .= 0
B[DimIndices(sub)] .+= sub
C[DimSelectors(sub)] .+= sub
@test A[DimIndices(sub)] == B[DimIndices(sub)] == C[DimIndices(sub)]
sub = A[2:4, 2:5]
C .= 0
C[DimSelectors(sub)] .+= sub
@test A[DimSelectors(sub)] == C[DimSelectors(sub)]
end

# @testset "Competing Wrappers" begin
# da = DimArray(ones(4), X)
# ta = TrackedArray(5 * ones(4))
Expand Down
28 changes: 24 additions & 4 deletions test/indexing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,25 @@ end

a = da[X([2, 1]), Y([2, 1])] # Indexing with array works
@test a == [4 3; 2 1]
end

@testset "dimindices and dimselectors" begin
@test da[DimIndices(da)] == da
da[DimIndices(da)[X(1)]]
da[DimSelectors(da)]
da[DimSelectors(da)[X(1)]]
da1 = da .* 0
da2 = da .* 0
da1[DimIndices(da)] += da
da2[DimSelectors(da)] += da
@test da == da1
@test da == da2
da1 *= 0
da2 *= 0
da1[DimIndices(da)] += da
da2[DimSelectors(da)] += da
@test da == da1
@test da == da2
end

@testset "selectors work" begin
Expand Down Expand Up @@ -371,7 +385,7 @@ end
end

@testset "setindex!" begin
da_set = copy(da)
da_set = deepcopy(da)
da_set[X(2), Y(2)] = 77
@test da_set == [1 2; 3 77]
da_set[X(1:2), Y(1)] .= 99
Expand Down Expand Up @@ -673,9 +687,15 @@ end
@test @inferred s_set[2, 2] === (one=9.0, two=10.0f0, three=11)
@test_throws ArgumentError s_set[CartesianIndex(2, 2)] = (seven=5, two=6, three=7)

s_set_mixed = deepcopy(s_mixed)
s_set_mixed[1, 1] = (one=9, two=10, three=11, four=12)
@test @inferred s_set_mixed[1, 1] === (one=9.0, two=10.0f0, three=11, four=12)
s_set_mixed1 = deepcopy(s_mixed)
s_set_mixed2 = deepcopy(s_mixed)
s_set_mixed3 = deepcopy(s_mixed)
s_set_mixed1[1, 1] = (one=9, two=10, three=11, four=12)
s_set_mixed2[X=1, Y=1] = (one=19, two=20, three=21, four=22)
s_set_mixed3[Y(1), X(1)] = (one=29, two=30, three=31, four=32)
@test @inferred s_set_mixed1[1] === (one=9.0, two=10.0f0, three=11, four=12)
@test @inferred s_set_mixed2[1] === (one=19.0, two=20.0f0, three=21, four=22)
@test @inferred s_set_mixed3[1] === (one=29.0, two=30.0f0, three=31, four=32)
end

@testset "Empty getindedex/view/setindex throws a BoundsError" begin
Expand Down
2 changes: 0 additions & 2 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ using DimensionalData, Test, Aqua, SafeTestsets
Aqua.test_project_extras(DimensionalData)
Aqua.test_stale_deps(DimensionalData)
Aqua.test_deps_compat(DimensionalData)



@time @safetestset "interface" begin include("interface.jl") end
@time @safetestset "metadata" begin include("metadata.jl") end
Expand Down

0 comments on commit d49dfc4

Please sign in to comment.