Make `levels` return a `CategoricalArray`

[nalimilan]

Having levels automatically unwrap CategoricalValues is a bit inconsistent with the fallback defined in DataAPI, which is equivalent to sort!(unique(x)). This is mostly historical, as levels was defined in CategoricalArrays first. But it can make some implementations more complex when the goal is to return a CategoricalArray (JuliaData/DataFrames.jl#3031 (comment)).

So it could make sense to make levels return a CategoricalArray in the next breaking release. This should be mostly transparent for users as long as they don't apply functions like string operations to levels. This could end up being inconvenient in some cases though. It would also force allocating a new array each time levels is called (as we probably don't want to store a CategoricalArray inside the CategoricalPool for that).

Cc: @bkamins

[bkamins]

Let us start with listing cases when levels is intended to be used and then make a decision.

[bkamins]

I have looked back at it. I see two issues:

• levels unwraps and user code might rely on this fact
• unique also unwraps; in-particular this means that sort!(unique(x)) returned unwrapped values:

julia> x = categorical(["a","b","c",missing])
4-element CategoricalArray{Union{Missing, String},1,UInt32}:
 "a"
 "b"
 "c"
 missing

julia> sort!(unique(x))
4-element Vector{Union{Missing, String}}:
 "a"
 "b"
 "c"
 missing

[nalimilan]

Yeah, unique should definitely not unwrap, I have code locally to fix this. I'm more hesitant about levels, since this function was originally defined in CategoricalArrays, precisely to unwrap. The fallback from DataAPI does not unwrap because there's nothing to unwrap for other types in general. As you said people may rely on unwrapping (though maybe not that much).

Unfortunately I don't remember what was the problem in DataFrames and the link I posted points to a conversation where it's not obvious either. Do you remember? If we don't have a use case maybe we'd better close this and only fix unique.

[bkamins]

As you said people may rely on unwrapping (though maybe not that much).

This is the only problem I see. Otherwise, I would make both unique and levels not unwrap.

As for DataFrames.jl:

• https://github.com/JuliaData/DataFrames.jl/blob/main/src/groupeddataframe/complextransforms.jl#L230 (we now compare values not CategoricalValues - but this is not a problem right? as CategoricalValues would also compare correctly)

I do not see any other use of levels in DataFrames.jl

[ablaom]

When we were developing MLJ, a levels that returned categorical values would have been more natural, but we worked around the behaviour as it was (ditto unique). I think levels and unique are used in dozens of separate packages in the MLJ ecosystem. I think you should do what you think is the most natural moving forward, but in any event, I'd be happy to run MLJ's ecosystem-wide integration tests if you can point me to PR with the new behaviour.

[nalimilan]

* https://github.com/JuliaData/DataFrames.jl/blob/main/src/groupeddataframe/complextransforms.jl#L230 (we now compare values not `CategoricalValues` - but this is not a problem right? as `CategoricalValues` would also compare correctly)

@bkamins Yeah it's fine, == will work in both cases.

But I don't understand what I meant with this:

But it can make some implementations more complex when the goal is to return a CategoricalArray (JuliaData/DataFrames.jl#3031 (comment)).

The discussion this links to doesn't seem related. And we use levels at https://github.com/JuliaData/DataFrames.jl/blob/900d016e7a5f7e306b512f6633320ec1d595920d/src/abstractdataframe/abstractdataframe.jl#L1495, but it doesn't seem having it not unwrap would change anything...

When we were developing MLJ, a levels that returned categorical values would have been more natural, but we worked around the behaviour as it was (ditto unique). I think levels and unique are used in dozens of separate packages in the MLJ ecosystem. I think you should do what you think is the most natural moving forward, but in any event, I'd be happy to run MLJ's ecosystem-wide integration tests if you can point me to PR with the new behaviour.

@ablaom Would you have links that present this use case?

[bkamins]

OK - you are right. To be sure 100% we do not miss anything some tests of fillcombinations should be done that check it.
It seems it is OK:

julia> df = DataFrame(x=categorical(1:2), y=categorical('a':'b'), z=categorical(["x", "y"]))
2×3 DataFrame
 Row │ x     y     z
     │ Cat…  Cat…  Cat…
─────┼──────────────────
   1 │ 1     a     x
   2 │ 2     b     y

julia> df2 = mapcols(x -> convert(Vector, x), df)
2×3 DataFrame
 Row │ x     y     z
     │ Cat…  Cat…  Cat…
─────┼──────────────────
   1 │ 1     a     x
   2 │ 2     b     y

julia> typeof.(eachcol(fillcombinations(df, [:x, :y])))
3-element Vector{DataType}:
 CategoricalVector{Int64, UInt32, Int64, CategoricalValue{Int64, UInt32}, Union{}} (alias for CategoricalArray{Int64, 1, UInt32, Int64, CategoricalValue{Int64, UInt32}, Union{}})
 CategoricalVector{Char, UInt32, Char, CategoricalValue{Char, UInt32}, Union{}} (alias for CategoricalArray{Char, 1, UInt32, Char, CategoricalValue{Char, UInt32}, Union{}})
 CategoricalVector{Union{Missing, String}, UInt32, String, CategoricalValue{String, UInt32}, Missing} (alias for CategoricalArray{Union{Missing, String}, 1, UInt32, String, CategoricalValue{String, UInt32}, Missing})

julia> typeof.(eachcol(fillcombinations(df2, [:x, :y])))
3-element Vector{DataType}:
 CategoricalVector{Int64, UInt32, Int64, CategoricalValue{Int64, UInt32}, Union{}} (alias for CategoricalArray{Int64, 1, UInt32, Int64, CategoricalValue{Int64, UInt32}, Union{}})
 CategoricalVector{Char, UInt32, Char, CategoricalValue{Char, UInt32}, Union{}} (alias for CategoricalArray{Char, 1, UInt32, Char, CategoricalValue{Char, UInt32}, Union{}})
 CategoricalVector{Union{Missing, String}, UInt32, String, CategoricalValue{String, UInt32}, Missing} (alias for CategoricalArray{Union{Missing, String}, 1, UInt32, String, CategoricalValue{String, UInt32}, Missing})

[nalimilan]

Actually I forgot that unique was fixed by #358. So we just need to decide whether levels has to change or not.

[ablaom]

| @ablaom Would you have links that present this use case?

Since levels unwraps, we defined our own classes that returned unwrapped levels. This is used in the constructor for UnivariateFinite, provided by CategoricalDistributions.jl. The constructors are defined here but perhaps the docstring already gives you the idea. A UnivariateFinite is basically a probability mass function for a distribution whose sample space is the (unwrapped) levels of a CategoricalVector.

But model interfaces themselves also use classes (and workarounds for existing unique behaviour). Here's an example: https://github.com/JuliaAI/MLJDecisionTreeInterface.jl/blob/274b2a907d39a5e8b238f5a7d679afd9dadf3bc1/src/MLJDecisionTreeInterface.jl#L45

[ablaom]

I also note here for the record that we are doing the classes workaround again in the new LearnAPI project, LearnAPIDataFrontEnds.jl. Here, however, I think we manage to keep classes private, ie. the implementers of model interfaces don't need it.

[nalimilan]

OK. See #425 for a possible implementation.

The drawback of this implementation is that we store a vector with one index for each level, which is kind of a waste (but the assumption in CategoricalArrays is that the number of levels is negligible). I considered having levels return the CategoricalPool itself, which already implements part of the AbstractArray interface. But this is tricky because with levels(x, skipmissing=false) we need to change the eltype to allow Missing. It's also simpler for users to get a CategoricalArray than a new type which is not obviously an array.