-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
keyword indexing fails for 0-dimensional DimArray #423
Comments
I thought what was missing was the following method julia> @inline Dimensions.dims2indices(::Tuple{}, I) = () But this has an unintended result. Indexing by a missing keyword then behaves like julia> x[c=1]
3
julia> ds = DimStack((; x, y));
julia> ds[c=1]
┌ Warning: (Dim{:c},) dims were not found in object
└ @ DimensionalData.Dimensions ~/.julia/packages/DimensionalData/K9D4P/src/Dimensions/primitives.jl:645
(x = 3, y = [-1.5090420829521751 -0.629637266507813 -1.1542533262114874; 1.78624724297272 -1.341032743849202 0.2629446089245767]) I would expect |
It is pretty weird I agree. But I would also have defined your missing method, expecting the result you got. We fill in missing dimensions with So I guess you are asking to special case this to not call That also feels pretty wierd to me,
Edit: another way to explain why I expect your result is that This keeps to to our mental model for arrays - that altering an object after Maybe what we want for |
Perhaps I have the wrong mental model of
That's good to know and could let me work around this. I should probably be |
It was hard to work out the behaviour when you index arrays with different dimensions all at the same time - because But from this discussion I think using regular Then we always have a Would that be better? |
If such a method existed that would indeed be useful. But IIUC, this could be implemented as noscalargetindex(s::AbstractDimStack; kwargs...) = copy(view(s; kwargs...)) so maybe it's better to just document that users who want this behavior should use this approach instead of having a special method? |
Ah wait, it seems julia> x = DimArray(randn(2), :a)
2-element DimArray{Float64,1} with dimensions: Dim{:a}
1 -0.753079
2 -0.502682
julia> ds = DimStack((; x))
DimStack with dimensions: Dim{:a}
and 1 layer:
:x Float64 dims: Dim{:a} (2)
julia> view(ds; a=1)
(x = fill(-0.753079230064672),) |
That's a problem here rather than in Base so easy fix. Maybe |
Ah, it seems to be due to this inconsistency: julia> x = DimArray(randn(2), :a);
julia> y = DimArray(randn(2, 3), (:b, :c));
julia> view(x; a=1) # a `DimArray` is not returned
0-dimensional view(::Vector{Float64}, 1) with eltype Float64:
-1.1244365106893395
julia> view(x, 1) # what the above dispatches to
0-dimensional view(::Vector{Float64}, 1) with eltype Float64:
-1.1244365106893395
julia> z = DimArray(fill(randn()), ()); # also a problem for 0-dim arrays
julia> view(z) # fine with no indices
0-dimensional DimArray{Float64,0}:
0.477376
julia> view(z, 1, 1) # or 2
0-dimensional DimArray{Float64,0}:
0.477376
julia> view(z, 1) # but not a DimArray with 1 index
0-dimensional view(::Vector{Float64}, 1) with eltype Float64:
0.4773758383436243
julia> view(y; b=1, c=1) # if we index all dimensions, a `DimArray` is still returned
0-dimensional DimArray{Float64,0}: and reference dimensions: Dim{:b}, Dim{:c}
0.364411
julia> view(y, 1, 1) # what the above dispatches to
0-dimensional DimArray{Float64,0}: and reference dimensions: Dim{:b}, Dim{:c}
0.364411 The issues seems to be this overload, DimensionalData.jl/src/array/indexing.jl Line 17 in 70f2aef
For more than one integer index, there's a DimensionalData.jl/src/array/indexing.jl Lines 43 to 49 in 70f2aef
I can open a PR fixing this. |
The text was updated successfully, but these errors were encountered: