8000 [MRG + 1] Label warning by kennethjmyers · Pull Request #7802 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG + 1] Label warning #7802

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jan 24, 2017
Merged

[MRG + 1] Label warning #7802

merged 13 commits into from
Jan 24, 2017

Conversation

kennethjmyers
Copy link

Reference Issue

Fixes #7751

What does this implement/fix? Explain your changes.

Raises warning if length of labels doesn't equal length of target names for a classification report. Also included a test for this

Any other comments?

jnothman
jnothman requested changes Nov 1, 2016
View reviewed changes
Copy link
Member
@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

Tests failing (unsurprisingly, given my comment)

sklearn/metrics/classification.py
@@ -1392,6 +1392,9 @@ class 2 1.00 0.67 0.80 3
else:
labels = np.asarray(labels)

if len(labels) != len(target_names):
Copy link
Member

Choose a reason for hiding this comment

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

But target_names is optional!

@jnothman jnothman mentioned this pull request Nov 1, 2016
Closed
amueller
amueller reviewed Nov 1, 2016
View reviewed changes
sklearn/metrics/classification.py
@@ -1392,6 +1392,9 @@ class 2 1.00 0.67 0.80 3
else:
labels = np.asarray(labels)

if target_names is not None and len(labels) != len(target_names):
warnings.warn("labels size does not match size of target_names")
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide the number of labels and number of target_names in the message?

Copy link
Author

Choose a reason for hiding this comment

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

sure

amueller
amueller reviewed Nov 1, 2016
View reviewed changes
sklearn/metrics/classification.py
@@ -1393,7 +1393,7 @@ class 2 1.00 0.67 0.80 3
labels = np.asarray(labels)

if target_names is not None and len(labels) != len(target_names):
warnings.warn("labels size, {}, does not match size of target_names, {}".format(len(labels), len(target_names))
Copy link
Member

Choose a reason for hiding this comment

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

The line is to long now. You can also run flake8 or pyflakes locally to find these errors without running the continuous integration. I would recommend installing a flake8 plugin for your editor / IDE.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the advice, I wasn't aware of these tools. I got normal flake8 working but I use pycharm mostly and can't find a plugin for it (and the built in inspector doesn't catch line length. Any advice on an editor/IDE I could use?

@amueller
Copy link
Member
amueller commented Nov 1, 2016

LGTM apart from pep8
8000

@amueller amueller changed the title Label warning [MRG + 1] Label warning Nov 1, 2016
@amueller
Copy link
Member
amueller commented Nov 1, 2016

@kennmyers https://blog.jetbrains.com/pycharm/2013/02/long-awaited-pep-8-checks-on-the-fly-improved-doctest-support-and-more-in-pycharm-2-7/

@kennethjmyers
Copy link
Author

@amueller why do the lines I write that are over the 79 characters throw errors, but lines that were already in the files and are also over 79 characters not throw errors?

examples of things that fail flake8 locally on my computer that I didn't write but do not fail here:

    assert_almost_equal(cohen_kappa_score(y1, y2, weights="linear"), .9412, decimal=4)
    assert_almost_equal(cohen_kappa_score(y1, y2, weights="quadratic"), .9541, decimal=4)

kennethjmyers
kennethjmyers commented Nov 2, 2016
View reviewed changes
Copy link
Author
@kennethjmyers kennethjmyers left a comment •
edited
Loading

Choose a reason for hiding this comment

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

made adjustments, please see latest commit

@lesteve
Copy link
Member
lesteve commented Nov 2, 2016

why do the lines I write that are over the 79 characters throw errors, but lines that were already in the files and are also over 79 characters not throw errors?

This is by design, we are only flake8-ing the lines that the PR changes. Look at https://github.com/scikit-learn/scikit-learn/blob/master/build_tools/travis/flake8_diff.sh for more details.

@lesteve lesteve mentioned this pull request Nov 2, 2016
Closed
@amueller amueller added this to the 0.19 milestone Nov 2, 2016
@jnothman jnothman merged commit 9bbe717 into scikit-learn:master Jan 24, 2017
sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017
@sergeyf
ENH warn in classification_report when target_names doesn't equal lab…
568c998 
…els size (scikit-learn#7802)

* Added warning for classification_report when target_names doesn't equal labels size and tests for such a case.
@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
Closed
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
@Sundrique
ENH warn in classification_report when target_names doesn't equal lab…
7eeeefd 
…els size (scikit-learn#7802)

* Added warning for classification_report when target_names doesn't equal labels size and tests for such a case.
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
@NelleV
ENH warn in classification_report when target_names doesn't equal lab…
0a77ce9 
…els size (scikit-learn#7802)

* Added warning for classification_report when target_names doesn't equal labels size and tests for such a case.
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
@paulha
ENH warn in classification_report when target_names doesn't equal lab…
86a9270 
…els size (scikit-learn#7802)

* Added warning for classification_report when target_names doesn't equal labels size and tests for such a case.
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
@maskani-moh
ENH warn in classification_report when target_names doesn't equal lab…
3ca3f06 
…els size (scikit-learn#7802)

* Added warning for classification_report when target_names doesn't equal labels size and tests for such a case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amueller amueller amueller left review comments

@jnothman jnothman jnothman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.19
Development

Successfully merging this pull request may close these issues.

raise warning if labels and target_names have different length in classification_report
4 participants
@kennethjmyers @amueller @lesteve @jnothman
0