rafaqz / DimensionalData.jl

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

`cat` is not type-inferrable #500

Open sethaxen opened 1 year ago

sethaxen commented 1 year ago

It seems that when using cat, Julia cannot infer the types of the resulting dimensions:

julia> using DimensionalData, Test

julia> foo(x, y) = cat(x, y; dims=Dim{:foo}([:c, :d]));

julia> da = DimArray(randn(3), Dim{:a})
3-element DimArray{Float64,1} with dimensions: Dim{:a}
 1   0.54291
 2  -0.0289045
 3   0.526266

julia> foo(da, da)
3×2 DimArray{Float64,2} with dimensions: 
  Dim{:a},
  Dim{:foo} Categorical{Symbol} Symbol[:c, :d] ForwardOrdered
   :c          :d
  0.54291     0.54291
 -0.0289045  -0.0289045
  0.526266    0.526266

julia> @inferred foo(da, da)
ERROR: return type DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:foo, DimensionalData.Dimensions.LookupArrays.Categorical{Symbol, Vector{Symbol}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.NoMetadata}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata} does not match inferred return type DimArray{Float64, 2, _A, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata} where _A<:Tuple
rafaqz commented 1 year ago

Yeah, it doesnt yet know the order or if the lookup is ordered at all.

You can (fully) specify the Categorical lookup manually and it should be type stable.

rafaqz commented 1 year ago

Yes its fine if you specify Categorical with the order keyword:

using DimensionalData.LookupArrays
bar(x, y) = cat(x, y; dims=Dim{:foo}(Categorical([:c, :d]; order=ForwardOrdered())))

julia> @inferred bar(da, da)
3×2 DimArray{Float64,2} with dimensions: 
  Dim{:a},
  Dim{:foo} Categorical{Symbol} Symbol[:c, :d] ForwardOrdered
   :c         :d
  1.6825     1.6825
  0.216932   0.216932
 -0.66003   -0.66003
sethaxen commented 11 months ago

@rafaqz it seems on v0.25 even specifying the lookup does not cause cat to be type-inferrable:

julia> using DimensionalData, Test

julia> using DimensionalData.LookupArrays

julia> bar(x, y) = cat(x, y; dims=Dim{:foo}(Categorical([:c, :d]; order=ForwardOrdered())))
bar (generic function with 1 method)

julia> da = DimArray(randn(3), Dim{:a})
3-element DimArray{Float64,1} with dimensions: Dim{:a}
 1  -0.214735
 2   0.395572
 3  -0.388922

julia> @inferred bar(da, da)
ERROR: return type DimArray{Float64, 2, Tuple{Dim{:a, NoLookup{Base.OneTo{Int64}}}, Dim{:foo, Categorical{Symbol, Vector{Symbol}, ForwardOrdered, NoMetadata}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, NoMetadata} does not match inferred return type Any
rafaqz commented 11 months ago

Making cat type stable when allowing it to still work on runtime-validated incorrect dims is pretty hard.

Maybe we need to separate out method dispatch for this specific case so its not mixed into a method with multiple return types.

A PR would help, I've spent too much time working on cat lately 😅