dim-aware broadcast of `Sampled` and `Categorical` dim does not error but probably should

[tiemvanderdeure]

MWE:

using DimensionalData
dv1 = rand(X(1:3))
dv2 = rand(X(Lookups.Categorical([1,2,3])))

bc = @d dv1 .* dv2
dims(bc)

returns an X dim with NoLookup. Probably this should not pass comparedims and just throw an error.

Multiplying a Sampled with a Projected always returns Sampled dims, I don't know if this is intentional?

dv3 = rand(X(Projected([1,2,3], crs = EPSG(4326))))
crs(@d dv3 .* dv1) # nothing

[rafaqz]

Projected is AbstractSampled so it falls back to Sampled as that's the best we can do in DD. I guess we can set up dispatch in Rasters so that Projected can "win" ? (So DD takes the abstract type methods, and other packages can own the subtypes if they want to. We probably need a way to resolve conflict like Base.BroadcastStyle, but it's pretty huge to do that)

And the other case is just pragmatic, they are both X at least.

Maybe we can have strictness levels?

[tiemvanderdeure]

So it is intentional? It was just surprising because if any of the other fields are different then it immediately throws an error. So an unordered and an ordered lookup cannot be broadcasted but a sampled and a categorical can. E.g.

julia> rand(X(Sampled(1:3, order = Lookups.Unordered()))) .* rand(X(1:3))
ERROR: DimensionMismatch: Lookups do not all have the same order: Unordered(), ForwardOrdered().
Stacktrace:
  [1] _failed_comparedims(t::DimensionalData.Dimensions.Throw{Nothing}, msg_intro::String)
    @ DimensionalData.Dimensions C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:569
  [2] _orderaction(err::DimensionalData.Dimensions.Throw{Nothing}, a::X{Sampled{…}}, b::X{Sampled{…}})
    @ DimensionalData.Dimensions C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:566
  [3] #_comparedims2#89
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:616 [inlined]
  [4] _comparedims2
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:604 [inlined]
  [5] _comparedims2
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:592 [inlined]
  [6] macro expansion
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:0 [inlined]
  [7] _comparedims_gen
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:576 [inlined]
  [8] _comparedims
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:574 [inlined]
  [9] comparedims
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\Dimensions\primitives.jl:534 [inlined]
 [10] _comparedims_broadcast
    @ C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\array\broadcast.jl:334 [inlined]
 [11] copy(bc::Base.Broadcast.Broadcasted{DimensionalData.DimensionalStyle{…}, Tuple{…}, typeof(*), Tuple{…}})
    @ DimensionalData C:\Users\tsh371\.julia\packages\DimensionalData\iHSqC\src\array\broadcast.jl:73
 [12] materialize(bc::Base.Broadcast.Broadcasted{DimensionalData.DimensionalStyle{…}, Nothing, typeof(*), Tuple{…}})
    @ Base.Broadcast .\broadcast.jl:867
 [13] top-level scope

[rafaqz]

Yeah, that sounds inconsistent.

Maybe partially intentional, as much as you can say that about a new interface over a combinatorial set of possibilities with unclear implications.

(Like I thought about it all enough to make most return types seem sensible, but not exhaustively, so you probably have a clearer picture when actually using that combination in practice)

[tiemvanderdeure]

I think 99% of cases where this happens this would be uninentional (as it was in the case where I stumbled across it). In those cases I think the best thing is to throw an error, so the user can fix whatever dim is broken (I unintentionally had a categorical month and a sampled month dim).

If hadn't even checked but with strict=false the same thing happens. In that case I would expect it to return whichever X dimension is first, which is also the behaviour with other non-matching dims with identical names.

[rafaqz]

Ok let's throw an error, and people can use strict=false if they need it