Skip to content

FIX improve error message with string-encoded target in metrics#18192

Merged
thomasjpfan merged 10 commits into
scikit-learn:masterfrom
glemaitre:consistent_error_pos_label
Sep 2, 2020
Merged

FIX improve error message with string-encoded target in metrics#18192
thomasjpfan merged 10 commits into
scikit-learn:masterfrom
glemaitre:consistent_error_pos_label

Conversation

@glemaitre

@glemaitre glemaitre commented Aug 19, 2020

Copy link
Copy Markdown
Member

This test makes sure that we fail with a human-understandable message when the target y_true or the predictions y_pred are encoded as strings.

@glemaitre glemaitre marked this pull request as draft August 19, 2020 08:23
@glemaitre glemaitre changed the title [WIP] TST make consistent error for metric having pos_label FIX improve error message with string-encoded target when used in metrics Aug 19, 2020
@glemaitre glemaitre marked this pull request as ready for review August 19, 2020 09:57
@glemaitre

glemaitre commented Aug 19, 2020

Copy link
Copy Markdown
Member Author

The failure in brier_score_loss is expected and should be handled properly after merging #18183

thomasjpfan
thomasjpfan reviewed Aug 25, 2020
View reviewed changes

@thomasjpfan thomasjpfan left a comment

Copy link
Copy Markdown
Member

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 @glemaitre !

Comment thread sklearn/metrics/_classification.py
Comment thread sklearn/metrics/_classification.py
Comment thread sklearn/metrics/tests/test_common.py Outdated
thomasjpfan
thomasjpfan reviewed Aug 29, 2020
View reviewed changes

@thomasjpfan thomasjpfan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Test still failing. (brier_score_loss sure likes using max() and min())

Comment thread sklearn/metrics/_ranking.py Outdated
@glemaitre glemaitre mentioned this pull request Aug 31, 2020
Open
@glemaitre

glemaitre commented Aug 31, 2020

Copy link
Copy Markdown
Member Author

I opened #18307 and comment out the test with brier_score_loss with a comment linked to the future discussion. I think this is the more sensible thing to do to have thing moving forward.

@glemaitre

glemaitre commented Aug 31, 2020

Copy link
Copy Markdown
Member Author

@thomasjpfan We need to convert into a list the label NumPy array because we will fall into the FutureWarning from NumPy and there is no easy solution there. Since it just make a copy of the labels, the overhead is not so bad.

thomasjpfan
thomasjpfan approved these changes Aug 31, 2020
View reviewed changes

@thomasjpfan thomasjpfan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor comment. Otherwise LGTM

Comment thread sklearn/metrics/tests/test_common.py Outdated
glemaitre and others added 2 commits September 2, 2020 10:18
@glemaitre @thomasjpfan
Apply suggestions from code review
7bc0799 
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@glemaitre
FIX syntax error
55420c5
@thomasjpfan thomasjpfan changed the title FIX improve error message with string-encoded target when used in metrics FIX improve error message with string-encoded target in metrics Sep 2, 2020
@thomasjpfan thomasjpfan merged commit a54cb47 into scikit-learn:master Sep 2, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@glemaitre @thomasjpfan @jayzed82
FIX improve error message with string-encoded target in metrics (scik…
bbaae0f 
…it-learn#18192)

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@StefanieSenger StefanieSenger mentioned this pull request Dec 17, 2025
Merged
4 tasks
This was referenced Jan 15, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort approved these changes
@thomasjpfan thomasjpfan thomasjpfan approved these changes

Assignees

No one assigned

Labels

module:metrics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@glemaitre @agramfort @thomasjpfan