DimensionalData.jl
DimensionalData.jl copied to clipboard
separate `set` and `unsafe_set` so that users can't end up with broken lookups
When I try to use At("Red") on a ForwardOrdered dimension this fails. I got the Dimension by using set on a Band dimension from GDAL but I can also reproduce this with plain DD calls. When I use
julia> dd = DimArray(rand(2,3), (Dim{:colors}(["Red", "Green"]), X(1:3)))
2×3 DimArray{Float64,2} with dimensions:
Dim{:colors} Categorical{String} String["Red", "Green"] ReverseOrdered,
X Sampled{Int64} 1:3 ForwardOrdered Regular Points
1 2 3
"Red" 0.492407 0.360465 0.149189
"Green" 0.215122 0.595209 0.985611
julia> dd1 = set(dd, X=>Dim{:Colors}(["Red", "Blue", "Green"]))
2×3 DimArray{Float64,2} with dimensions:
Dim{:colors} Categorical{String} String["Red", "Green"] ReverseOrdered,
Dim{:Colors} Sampled{String} String["Red", "Blue", "Green"] ForwardOrdered Regular Points
"Red" "Blue" "Green"
"Red" 0.492407 0.360465 0.149189
"Green" 0.215122 0.595209 0.985611
julia> dd1[Colors=At("Red")]
selval = "Red"
i = 3
ERROR: ArgumentError: Red not found in ["Red", "Blue", "Green"]
Stacktrace:
[1] _selvalnotfound(lookup::DimensionalData.Dimensions.LookupArrays.Sampled{…}, selval::String)
@ DimensionalData.Dimensions.LookupArrays ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:202
[2] _selnotfound_or_nothing(err::DimensionalData.Dimensions.LookupArrays._True, lookup::DimensionalData.Dimensions.LookupArrays.Sampled{…}, selval::String)
@ DimensionalData.Dimensions.LookupArrays ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:200
[3] at(::DimensionalData.Dimensions.LookupArrays.ForwardOrdered, ::DimensionalData.Dimensions.LookupArrays.Regular{…}, lookup::DimensionalData.Dimensions.LookupArrays.Sampled{…}, selval::String, atol::Nothing, rtol::Nothing; err::DimensionalData.Dimensions.LookupArrays._True)
@ DimensionalData.Dimensions.LookupArrays ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:183
[4] at
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:159 [inlined]
[5] #at#24
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:143 [inlined]
[6] at
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:142 [inlined]
[7] selectindices
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/LookupArrays/selector.jl:120 [inlined]
[8] _dims2indices
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/Dimensions/indexing.jl:114 [inlined]
[9] macro expansion
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/Dimensions/indexing.jl:56 [inlined]
[10] _dims2indices
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/Dimensions/indexing.jl:56 [inlined]
[11] dims2indices
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/Dimensions/indexing.jl:51 [inlined]
[12] dims2indices
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/Dimensions/indexing.jl:34 [inlined]
[13] #getindex#58
@ ~/Documents/PyramidScheme/dev/DimensionalData/src/array/indexing.jl:47 [inlined]
[14] top-level scope
@ REPL[56]:1
Some type information was truncated. Use `show(err)` to see complete types.
Oh, yeah... weird, I can reproduce.
Its not forward ordered tho, so searchsorted wont work.
Probably set should error
I expected to fully replace the Dimension so that it has an order that fits to the values. If set errors how would you change a dimension?
Maybe we need to let you set both in the same call so there is never an incorrect state.
The problem with auto-detecting the order in set is it would make every change to the lookup values type-unstable. Normally set is a compile-time op or close to it so you can use it freely in hot paths. It would be unfortunate to lose that. format does these things but its never expected to be type stable and only happens in user called constructors.
But I've been thinking we may need to split set into unsafe_set and set. unsafe_set wouldn't check anything that would lead to type instability, and would be safe for use in hot paths, while set would be safe for users to use in the REPL and would avoid problems like this one.