8000 DOC Add examples to PrecisionRecall and ConfusionMatrix Display by pardeep-singh · Pull Request #17492 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC Add examples to PrecisionRecall and ConfusionMatrix Display #17492

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

Conversation

pardeep-singh
Copy link
Contributor

Reference Issues/PRs

References #3846

What does this implement/fix? Explain your changes.

This PR adds examples to PrecisionRecallDisplay and ConfusionMatrixDisplay classes.

Any other comments?

CC @violetr #dataumbrella

@amueller
Copy link
Member
amueller commented Jun 6, 2020

cc @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 @pardeep-singh !


Examples
--------
>>> import matplotlib.pyplot as plt
Copy link
Member

Choose a reason for hiding this comment

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

Some of our CI do not have matplotlib installed.

Suggested change
>>> import matplotlib.pyplot as plt

>>> cm = confusion_matrix(y_test, predictions, labels=clf.classes_)
>>> disp = ConfusionMatrixDisplay(confusion_matrix=cm,
>>> display_labels=clf.classes_)
>>> disp.plot()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> disp.plot()
>>> disp.plot() # doctest: +SKIP


Examples
--------
>>> import matplotlib.pyplot as plt
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> import matplotlib.pyplot as plt

Comment on lines 60 to 61
>>> viz = PrecisionRecallDisplay(precision=precision, recall=recall)
>>> viz.plot()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> viz = PrecisionRecallDisplay(precision=precision, recall=recall)
>>> viz.plot()
>>> disp = PrecisionRecallDisplay(precision=precision, recall=recall)
>>> disp.plot() # doctest: +SKIP

@pardeep-singh
Copy link
Contributor Author

Hey @thomasjpfan , I have made the suggested changes. Can you please take a look at the latest commit? Thanks!

Comment on lines 51 to 53
>>> X_train, X_test, y_train, y_test = train_test_split(X,
>>> y,
>>> random_state=0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> X_train, X_test, y_train, y_test = train_test_split(X,
>>> y,
>>> random_state=0)
>>> X_train, X_test, y_train, y_test = train_test_split(X, y,
... random_state=0)

Comment on lines 52 to 54
>>> X_train, X_test, y_train, y_test = train_test_split(X,
>>> y,
>>> random_state=0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> X_train, X_test, y_train, y_test = train_test_split(X,
>>> y,
>>> random_state=0)
>>> X_train, X_test, y_train, y_test = train_test_split(X, y,
... random_state=0)

@pardeep-singh
Copy link
Contributor Author

Hey @thomasjpfan, I have made the latest suggested changes. Can you please check these? Also, I am not able to understand build failures. Can you please help me understand these?
Thanks!

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.

This should fix the failing tests.

You can run the docstring test locally by:

pytest sklearn/metrics/_plot/confusion_matrix.py

>>> from sklearn.svm import SVC
>>> X, y = make_classification(random_state=0)
>>> X_train, X_test, y_train, y_test = train_test_split(X, y,
>>> random_state=0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> random_state=0)
... random_state=0)

>>> predictions = clf.predict(X_test)
>>> cm = confusion_matrix(y_test, predictions, labels=clf.classes_)
>>> disp = ConfusionMatrixDisplay(confusion_matrix=cm,
>>> display_labels=clf.classes_)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> display_labels=clf.classes_)
... display_labels=clf.classes_)

>>> from sklearn.svm import SVC
>>> X, y = make_classification(random_state=0)
>>> X_train, X_test, y_train, y_test = train_test_split(X, y,
>>> random_state=0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> random_state=0)
... random_state=0)

@pardeep-singh
Copy link
Contributor Author

@thomasjpfan Thanks, Verified by running the tests locally. I had to add the return value to clf.fit(X_train, y_train) statement also. Builds should pass on the latest commit. 🙏

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 @pardeep-singh !

LGTM

@thomasjpfan thomasjpfan changed the title Add examples to PrecisionRecallDisplay and ConfusionMatrixDisplay classes DOC Add examples to PrecisionRecall and ConfusionMatrix Display Jun 7, 2020
@thomasjpfan thomasjpfan merged commit 3f10622 into scikit-learn:master Jun 7, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
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.

3 participants
0