Fix a warning when constructing UnivariateFinite with CategoricalArrays v1#314
Conversation
| if target_levels isa AbstractArray{<:CategoricalValue} | ||
| return MMI.UnivariateFinite(target_levels, pred) | ||
| else | ||
| return MMI.UnivariateFinite(target_levels, pred, pool=missing, ordered=fitresult.info[:target_isordered]) |
There was a problem hiding this comment.
Maybe it's better (if it has not already been done) to make CategoricalArrays 1.0 the lower compat bound. It's the first "official" stable release, after all. Then this second case can be eliminated, right?
There was a problem hiding this comment.
Yes, that would be simpler.
In my opinion, the only reason against it would be that users who depend on packages that still only support CategoricalArrays <1 would be unable to update to new versions of EvoTrees. But I think that’s not too much of a problem.
What do you think?
There was a problem hiding this comment.
My vote is to dump CategoricalArrays <1, but I think @jeremiedb should chime in here.
There was a problem hiding this comment.
I'm favourable to adopt a compat for CategoricalArrays = "1", but I think MLJModelInterface should first be updated to include v1 support:
https://github.com/JuliaAI/MLJModelInterface.jl/blob/b587c6eca489aba288d056b31bad594cfa2b1711/Project.toml#L15
@ablaom Do you have any objection pushing such update?
|
Sorry I'm completing a trip in Peru, will look at this in 2 days! |
|
@jeremiedb CategoricalArrays is only a test dependency of MLJModelInterface. It should not block using MLJModelInterface with any package requiring CategoricalArrays 1.0, as I can confirm, as the latest version of MLJ runs with the latest CategoricalArrays. That said, I've have extended the compat at MLJModelInterface here, which should be merged today. |
|
This PR now restricts CategoricalArrays to v1. |
|
@sostock In order to avoid other warnings if the target variable is not a categorical vector (since EvoTrees has been supporting String, Bool and Int as valid target variable types for Classification), I'd also force the feature_levels to be <: CategoricalValue. |
|
You can just merge #315, since it includes the changes from this PR. |
|
Release v0.18.2 is now available; let me know if you still encounter any issue. |
3 participants
After updating to MLJ v0.22, I noticed a lot of warnings being printed when using
EvoTreeClassifierthrough MLJ. This is because MLJ v0.22 uses CategoricalArrays v1.When using CategoricalArrays v1,
fitresult.info[:target_levels]is aCategoricalVector. In this case, theUnivariateFiniteconstructor prints two warnings, because the keyword arguments are redundant:https://github.com/JuliaAI/CategoricalDistributions.jl/blob/730c5df2f61810e7f8b83a53f68b6d745e92e4d1/src/types.jl#L385-L394
This PR removes the warnings by not supplying the keyword arguments when
target_levelsis aCategoricalVector.