rafaqz / DimensionalData.jl

Named dimensions and indexing for julia arrays and other data
https://rafaqz.github.io/DimensionalData.jl/stable/
MIT License
280 stars 41 forks source link

Dimensions.lookup fails if the lookup is `Colon` #454

Open sethaxen opened 1 year ago

sethaxen commented 1 year ago

Cases like this fail. I think though that they should work?

julia> Dimensions.lookup(X())
ERROR: MethodError: no method matching lookup(::X{Colon})

Closest candidates are:
  lookup(::Union{Val{<:DimensionalData.Dimensions.Dimension}, Type{<:DimensionalData.Dimensions.Dimension}})
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:175
  lookup(::DimensionalData.Dimensions.AnonDim)
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:351
  lookup(::DimensionalData.Dimensions.Dimension{<:AbstractArray})
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:174
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[70]:1

julia> Dimensions.lookup(Dim{:foo}())
ERROR: MethodError: no method matching lookup(::Dim{:foo, Colon})

Closest candidates are:
  lookup(::Union{Val{<:DimensionalData.Dimensions.Dimension}, Type{<:DimensionalData.Dimensions.Dimension}})
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:175
  lookup(::DimensionalData.Dimensions.AnonDim)
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:351
  lookup(::DimensionalData.Dimensions.Dimension{<:AbstractArray})
   @ DimensionalData ~/.julia/packages/DimensionalData/6v7CY/src/Dimensions/dimension.jl:174
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[71]:1
rafaqz commented 1 year ago

Yeah, it's on Dim{<:AbstractArray}.

For better or worse. The reason was because dims are used for so many things it's a potential footgun having lookup return contents. So it will only ever return something indexable, usually a Lookup.

val gives you the contents of the Dimension whatever it is. So does Base.parent.

Clearly this all happened in various phases as changes were made (with no PR reviews), and I was never totally happy with it. But that's how it works...

sethaxen commented 1 year ago

What threw me off is that val(Dim{:foo}()) works but lookup(Dim{:foo}()) doesn't while val(Dim{:foo}) doesn't work while lookup(Dim{:foo}) does.

rafaqz commented 9 months ago

I think that lookup on Dim{:foo} should be lookup_type