MNT clean-up deprecations for 1.7: multi_class in LogisticRegression#31241
Conversation
|
First of all, sorry for this mistake. I would actually dare to carry out the deprecation in 1.7 and see if anybody complains. I have heard so many complaints about scikit-learn, but never about deprecations and their settlement . |
Well I think I reviewed your PR so we share it :)
Maybe because we're cautious about backward compatibility and because we take care to break their code without notice 😉
It crossed my mind, but I think it's better to do the right thing. It could be a pain for some users while keeping the deprecated code 1 more release for us is not an major issue (it might delay some refactorings in logistic regression if planned though). |
We should still add to changelog so people know about it. One cycle for a deprecation warning is shorter than normal.
Done |
lorentzenchr
left a comment
There was a problem hiding this comment.
@jeremiedbb It seems I forgot to update the solver table in the docstring of LogisticRegression: newton-cholesky now supports multiclass!!!
Could you update that here?
|
@jeremiedbb please review this minor documentation correction. |
On scikit-learn >= 1.7, using the liblinear solver for multiclass classification with a one-versus-rest (ovr) scheme is deprecated. See: scikit-learn/scikit-learn#31241
When cleaning-up the deprecation of the
multi_classparameter, see #28703, I found that we haven't been raising a warning the last 2 versions for the following situation:In this setting it falls back to OVR because liblinear doesn't support multinomial. However the deprecation of
multi_classimplied that all multiclass problems would be fitted using the multinomial loss and that an error would be raised if a solver doesn't support it. SeeThat's an issue because doing it for 1.7 would break code that was working until now without any FutureWarning.
A pragmatic solution is to delay the deprecation clean-up and raise a warning for this specific case for 1 more release (I don't think we really need 2 here). This is what I implemented in this PR. What do you think @lorentzenchr @ogrisel ?
Do you think it's worth a new specific changelog entry ? I feel like the one in v1.5.rst is enough.