Broken integration with StatsBase on 0.28 (or 0.29)!!!

[Datseris]

I am reporting a BREAKING behavior that has occured somewhere between 0.28 and 0.29, and does not occur on 0.27.

Here is the MWE:

using DimensionalData
using StatsBase

A = DimArray(rand(50, 31), (X(), Y(10.0:40.0)))
W = weights(rand(size(A)...))
mean(A, W)

ERROR: TypeError: in keyword argument dims, expected Union{Colon, Int64}, got a value of type Tuple{Int64, Int64}
Stacktrace:
 [1] mean(A::DimMatrix{…}, w::Weights{…}; dims::Function)
   @ DimensionalDataStatsBase C:\Users\datse\.julia\packages\DimensionalData\omXy6\ext\DimensionalDataStatsBase.jl:10
 [2] mean(A::DimMatrix{…}, w::Weights{…})
   @ DimensionalDataStatsBase C:\Users\datse\.julia\packages\DimensionalData\omXy6\ext\DimensionalDataStatsBase.jl:9
 [3] top-level scope
   @ REPL[25]:1
Some type information was truncated. Use `show(err)` to see complete types.

I am surprised that such a thing was not caught in the test suite. I assume there are no tests that test the integration of DimensionalData.jl and StatsBase.jl via the extension module? I'd love to fix this issue but unfortunately I assume an internal change happened to DD between 0.27 to 0.29 that I have not understood.

EDIT: I am sorry about my tone, which was accussatory. I was very frustrated to find this out.

[rafaqz]

There are some tests, no need to assume ;). But maybe not enough. Looks like we are missing the test for no dimensions.

DimensionalData.jl/test/ecosystem.jl

Lines 83 to 87 in 20b42df

	@testset "StatsBase" begin
	da = rand(X(1:10), Y(1:3))
	@test mean(da, weights([0.3,0.3,0.4]); dims=Y) == mean(parent(da), weights([0.3,0.3,0.4]); dims=2)
	@test sum(da, weights([0.3,0.3,0.4]); dims=Y) == sum(parent(da), weights([0.3,0.3,0.4]); dims=2)
	end

@tiemvanderdeure

[tiemvanderdeure]

Probably this PR introduced that bug? #909

[rafaqz]

So the problem is dimnum now always returned numbers, as the old default was broken in some edge cases.

So now instead of passing through the Colon : its converting it to (1, 2). But this is breaking the StatsBase functions. I changed this everywhere else for Base functions but forgot these extension methods because I also had earlier neglected to write the test without any arguments.

We simply need to check if dims isa Colon like this (not checked):

function Statistics.mean(A::AbstractDimArray, w::StatsBase.AbstractWeights; dims=:)
    data = if dims isa Colon
        mean(parent(A), w; dims=:)
    else
        mean(parent(A), w; dims=dimnum(A, dims))
    end
    return rebuild(A, data, DD.reducedims(A, dims))
end

In this and the 2 other methos, here:

DimensionalData.jl/ext/DimensionalDataStatsBase.jl

Lines 9 to 12 in 20b42df

	function Statistics.mean(A::AbstractDimArray, w::StatsBase.AbstractWeights; dims=:)
	data = mean(parent(A), w; dims=dimnum(A, dims))
	return rebuild(A, data, DD.reducedims(A, dims))
	end

I will be out on the snow trekking for the next few days, so if anyone else want to do it it should be pretty straighforward.

[tiemvanderdeure]

Why should dimnum not just return a colon? The behaviour isn't documented anyway, the docstring just says that dims can be a dimension or a dimtuple.

For mean without weights we solve this by dispatching on the internal _mean

[rafaqz]

It breaks a few other things, and I figured the name really suggests that it returns numbers ;)