-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
TST add binary and multiclass test for scorers #18904
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
TST add binary and multiclass test for scorers #18904
Conversation
…orer in _scorer.py, only 'neg_log_loss'
…tic regression model
…tratified test_train_split to ensure train and tests sets capture all classes.
…ling due to unrelated things [doc skip].
…ling due to unrelated things [doc skip].
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.
Sorry for the delay for reviewing. Some first thought regarding how to refactor the tests.
…fication_scores test to test_classificatio_binary_scores
…tion_binary_scores can be parameterized
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.
It looks good. Only a couple of nitpicks.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.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.
ping @thomasjpfan |
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.
Thank you for the PR @efiegel !
This is a good step forward. In a follow up PR, we should come up with a way to keep these lists up to date.
…on-tests fetched updates from main
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
Reference Issues/PRs
Resolves #12310 by adding more tests for multiclass classification tasks.
What does this implement/fix? Explain your changes.
Added tests for a multiclass DecisionTreeClassifier and LogisticRegression classifier on all scorers except binary-only scorers. Also removed log_loss from the list of CLF_SCORERS since there is no log_loss scorer in _scorer.py, only neg_log_loss.
Any other comments?
@amueller was this close to what you had in mind? This captures all new scorers added by @thomasjpfan in #12789.