-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
FIX add support for multilabel classification in RidgeClassifier* #19869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@agramfort Do you have an idea of a good test to check that the classification is going well in But maybe there is something better to be tested. |
OK this is ready for a review. Maybe @agramfort if you can have a look at this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides LGTM
Thx @glemaitre
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll let @lorentzenchr or @rth do the second review. Thx @glemaitre
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, but I need to re-read this more carefully a second time.
Thanks for this PR!
I address the comment @rth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good overall, just a few minor comments
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @glemaitre !
…ikit-learn#19869) Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
…ikit-learn#19869) Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
…9869) Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
closes #19858
closes #19882
build on the top of #19859
This PR fixes the
predict
method inRidgeClassifierCV
and enables support forRidgeClassifier
when the target is a multilabel indicator matrix.The common tests are using #19859. The change in this PR should be limited to
ridge.py
andtest_ridge.py
once #19859 is merged.