Skip to content

DOC Add examples to PrecisionRecall and ConfusionMatrix Display#17492

Merged
thomasjpfan merged 7 commits intoscikit-learn:masterfrom
pardeep-singh:doc_add_metrics_examples
Jun 7, 2020
Merged

DOC Add examples to PrecisionRecall and ConfusionMatrix Display#17492
thomasjpfan merged 7 commits intoscikit-learn:masterfrom
pardeep-singh:doc_add_metrics_examples

Conversation

@pardeep-singh
Copy link
Copy Markdown
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
Copy Markdown
Member

amueller commented Jun 6, 2020

cc @thomasjpfan

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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