Skip to content

DOC add link to plot_confusion_matrix example in confusion_matrix.py#30949

Merged
marenwestermann merged 3 commits intoscikit-learn:mainfrom
arjun-sundar3363:doc-add-example-confusion-matrix
Apr 14, 2025
Merged

DOC add link to plot_confusion_matrix example in confusion_matrix.py#30949
marenwestermann merged 3 commits intoscikit-learn:mainfrom
arjun-sundar3363:doc-add-example-confusion-matrix

Conversation

@arjun-sundar3363
Copy link
Copy Markdown
Contributor

@arjun-sundar3363 arjun-sundar3363 commented Mar 5, 2025

[DOC] Add link to plot_confusion_matrix example in sklearn/metrics/_plot/confusion_matrix.py

This PR adds a reference link to the plot_confusion_matrix example in sklearn/metrics/_plot/confusion_matrix.py for better documentation accessibility.

Changes:

  • Added a direct link to the relevant example.

Testing:

  • Verified documentation builds correctly.
  • Ran local tests successfully.

Towards #30621.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 5, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: b135cd7. Link to the linter CI: here

Copy link
Copy Markdown
Member

@virchan virchan 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, @Nujra40!

I have a few comments.

@arjun-sundar3363 arjun-sundar3363 force-pushed the doc-add-example-confusion-matrix branch 3 times, most recently from 360bc67 to 1b7c8af Compare March 7, 2025 10:08
@arjun-sundar3363 arjun-sundar3363 requested a review from virchan March 7, 2025 17:00
@arjun-sundar3363 arjun-sundar3363 force-pushed the doc-add-example-confusion-matrix branch from 1b7c8af to 87120c5 Compare March 8, 2025 17:11
@arjun-sundar3363
Copy link
Copy Markdown
Contributor Author

Hi @virchan I have made the changes, Can I get a review

Copy link
Copy Markdown
Member

@virchan virchan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @Nujra40!

@adrinjalali, @marenwestermann, would you like to have a look and merge this?

@arjun-sundar3363 arjun-sundar3363 force-pushed the doc-add-example-confusion-matrix branch from 87120c5 to 88facc2 Compare March 19, 2025 12:18
Copy link
Copy Markdown
Member

@marenwestermann marenwestermann left a comment

Choose a reason for hiding this comment

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

The function confusion_matrix where the example is currently linked according to this PR is actually not used in the example. The method which is used is `ConfusionMatrixDisplay.from_estimator. So the example needs to be linked there.

@arjun-sundar3363 arjun-sundar3363 force-pushed the doc-add-example-confusion-matrix branch from 88facc2 to 0e4347a Compare April 7, 2025 16:27
@arjun-sundar3363
Copy link
Copy Markdown
Contributor Author

Hi @marenwestermann I have made the requested changes. Are there any further changes required?

@arjun-sundar3363 arjun-sundar3363 changed the title DOC add link to plot_confusion_matrix example in _classification.py DOC add link to plot_confusion_matrix example in confusion_matrix.py Apr 8, 2025
Copy link
Copy Markdown
Member

@marenwestermann marenwestermann 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 your contribution @Nujra40. Looks good to me, I just added a blank line above your documentation and merged the latest changes from the main branch into your branch. Note for the future that force pushing shouldn't be required, a normal git push is sufficient.

@marenwestermann marenwestermann merged commit fae33fa into scikit-learn:main Apr 14, 2025
36 checks passed
lucyleeow pushed a commit to EmilyXinyi/scikit-learn that referenced this pull request Apr 23, 2025
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