Skip to content

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

Merged
merged 18 commits into from
Jan 28, 2021
Merged

TST add binary and multiclass test for scorers #18904

merged 18 commits into from
Jan 28, 2021

Conversation

efiegel
Copy link
Contributor

@efiegel efiegel commented Nov 25, 2020

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.

…tratified test_train_split to ensure train and tests sets capture all classes.
@efiegel efiegel changed the title Binary classification tests [MRG] Binary classification tests Nov 25, 2020
Copy link
Member

@glemaitre glemaitre left a 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.

@glemaitre glemaitre changed the title [MRG] Binary classification tests TST add binary and multiclass test for scorers Dec 19, 2020
Copy link
Member

@glemaitre glemaitre left a 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>
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@glemaitre
Copy link
Member

ping @thomasjpfan

Copy link
Member

@thomasjpfan thomasjpfan left a 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.

Base automatically changed from master to main January 22, 2021 10:53
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomasjpfan thomasjpfan merged commit ff2e52d into scikit-learn:main Jan 28, 2021
@glemaitre glemaitre mentioned this pull request Apr 22, 2021
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_score_objects only tests on binary classification tasks
3 participants