Improve array promotion rules #387
Conversation
PR #384 added promotion rules that recursively promote the eltype and reftype, but this has the drawback that the resulting type is often not a supertype of the input types, forcing a conversion to a `CategoricalArray` which may have a `Union` type. Instead, do the same as `Array` and `AbstractArray` fallbacks, which simply call `promote_typejoin` if eltypes do not match.
| @test promote_type(CategoricalVector{Int}, | ||
| CategoricalVector{String}) == | ||
| CategoricalVector{Union{Int, String}} | ||
| @test promote_type(CategoricalVector{Int, UInt32}, | ||
| CategoricalVector{String, UInt32}) == | ||
| CategoricalVector{Union{Int, String}, UInt32} | ||
| @test promote_type(CategoricalArray{Int, UInt32}, | ||
| CategoricalArray{String, UInt32}) == | ||
| CategoricalArray{Union{Int, String}, UInt32} | ||
| @test promote_type(CategoricalVector{Int, UInt32}, | ||
| CategoricalMatrix{String, UInt32}) == | ||
| CategoricalArray{Union{Int, String}} | ||
| @test promote_type(CategoricalVector{Int, UInt8}, | ||
| CategoricalVector{String, UInt16}) == | ||
| CategoricalVector{Union{Int, String}, UInt16} | ||
|
|
||
| @test promote_type(CategoricalVector{Int8}, | ||
| CategoricalVector{Float64}) == | ||
| CategoricalVector{Float64} | ||
| @test promote_type(CategoricalVector{Int8, UInt32}, | ||
| CategoricalVector{Float64, UInt32}) == | ||
| CategoricalVector{Float64, UInt32} | ||
| @test promote_type(CategoricalArray{Int8, UInt32}, | ||
| CategoricalArray{Float64, UInt32}) == | ||
| CategoricalArray{Float64, UInt32} | ||
| @test promote_type(CategoricalVector{Int8, UInt32}, | ||
| CategoricalMatrix{Float64, UInt32}) == | ||
| CategoricalArray{Float64} | ||
| @test promote_type(CategoricalVector{Int8, UInt8}, | ||
| CategoricalVector{Float64, UInt16}) == | ||
| CategoricalVector{Float64, UInt16} |
There was a problem hiding this comment.
Unfortunately this PR isn't enough to get these two work. CategoricalVector{T, R} isn't a concrete type, and even if the element type of such arrays is completely determined by T and R, the type system doesn't know that, which makes promote_rule and el_same above fail to return a precise type. It is probably fixable, but that's not trivial either. So for now instead of changing these tests to use verbose CategoricalArray{T, R, N, V, C U} types, I rely on array tests below, which are more compact and equivalent.
What do you think? Maybe it's worth supporting promotion of short CategoricalArray{T, N, R} forms, if only to limit confusing users (who will never write all type parameters when checking things manually)?
PR #384 added promotion rules that recursively promote the eltype and reftype, but this has the drawback that the resulting
type is often not a supertype of the input types, forcing a conversion to a
CategoricalArraywhich may have aUniontype.Instead, do the same as
ArrayandAbstractArrayfallbacks, which simply callpromote_typejoinif eltypes do not match.