Fix ambiguity error on construction from ChainedVector
#369
Conversation
src/CategoricalArrays.jl
Outdated
| DataAPI.defaultarray(::Type{CategoricalValue{T, R}}, N) where {T, R} = | ||
| CategoricalArray{T, N, R} | ||
| DataAPI.defaultarray(::Type{Union{CategoricalValue{T, R}, Missing}}, N) where {T, R} = | ||
| CategoricalArray{Union{T, Missing}, N, R} |
There was a problem hiding this comment.
why are these definitions linked to JSON? I would assume they make sense in general.
There was a problem hiding this comment.
Woops. Unfortunately tests don't catch this as JSON is loaded earlier...
| CategoricalArray{Union{T, Missing}, N, R} | ||
| end | ||
|
|
||
| @require RecipesBase="3cdcf5f2-1ef4-517c-9805-6587b60abb01" @eval begin |
There was a problem hiding this comment.
it would be good to cover this with tests if it is not overly complex.
There was a problem hiding this comment.
It's tested AFAICT. Or do you mean something else?
There was a problem hiding this comment.
Just coveage test was signaling it is not tested. That is why I asked.
There was a problem hiding this comment.
You mean line 50 at https://github.com/JuliaData/CategoricalArrays.jl/runs/3633298660? That's probably a bug, as other lines in the block appear to be covered. I guess a macro within two macro calls is too complex for coverage checks.
src/CategoricalArrays.jl
Outdated
| copyto!(dest::CatArrOrSub{<:Any, 1}, dstart::Union{Signed, Unsigned}, | ||
| src::SentinelArrays.ChainedVector, sstart::Union{Signed, Unsigned}, | ||
| n::Union{Signed, Unsigned}) = | ||
| invoke(copyto!, Tuple{AbstractArray, Union{Signed, Unsigned}, |
|
I understand that the proper code blocks will be executed both when:
Right? |
|
Yes, at least that's my understanding. :-) |
a4741d0 to
7904fc9
Compare
Fix #361 adding special methods for
ChainedVector, and make as many dependencies as possible optional using Requires.jl.@time using CategoricalArrayson a fresh session goes down from 0.26s to 0.18s.