rafaqz / DimensionalData.jl

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

can't specify Unordered for custom Dimension #594

Closed bjarthur closed 6 months ago

bjarthur commented 6 months ago

order=Unordered() works for X:

julia> using DimensionalData

julia> import DimensionalData: Unordered

julia> fill(NaN, X(1:3, order=Unordered()))
3-element DimArray{Float64,1} with dimensions: 
  X Sampled{Int64} 1:3 Unordered Regular Points
 1  NaN
 2  NaN
 3  NaN

but not my custom dim Foo:

julia> DimensionalData.@dim Foo

julia> fill(NaN, Foo(1:3, order=Unordered()))
ERROR: UndefVarError: `AutoLookup` not defined
Stacktrace:
 [1] Foo(val::UnitRange{Int64}; kw::@Kwargs{order::Unordered})
   @ Main ~/.julia/packages/DimensionalData/R7veM/src/Dimensions/dimension.jl:428
 [2] top-level scope
   @ REPL[5]:1

unless i use the Dim{:Foo} syntax:

julia> fill(NaN, Dim{:Foo}(1:3, order=Unordered()))
3-element DimArray{Float64,1} with dimensions: 
  Dim{:Foo} Sampled{Int64} 1:3 Unordered Regular Points
 1  NaN
 2  NaN
 3  NaN
rafaqz commented 6 months ago

Damn thats a bug with keyword handling maybe.

Btw im thinking to delete the custom dims anyway soon and switch to just Dim...

bjarthur commented 6 months ago

oh no! i find the concise syntax of custom dims nice. but, i guess it would be (syntactically) equivalent to define const Foo = Dim{:Foo} instead of using DimensionalData.@dim Foo, right? show would display Dim{:Foo} but programmatically you could still use e.g. fill(NaN, Foo(1:3, order=Unordered())).

rafaqz commented 6 months ago

Yeah const instead.

And we make traits to detect plot axis rather than inheritance like we have now.

We can also make show nicer and not show the Dim{} part most if the time.

rafaqz commented 6 months ago

Ah this is just a macro scope issue. Will fix.

bjarthur commented 6 months ago

thanks for fixing! but, i'm wondering now whether to use const or @dim. what are the tradeoffs? is there a discussion somewhere on your plan to remove custom dims?

rafaqz commented 6 months ago

It shouldn't make much difference - I will just change the macro to create the const definition and it will keep working with very little disruption for most things. I'm hoping e.g. Rasters.jl wont need to do anything with the update even though it uses the @dims macro.