Skip to content

DOC add read more tagline for contingency matrix metric#30666

Merged
ArturoAmorQ merged 4 commits intoscikit-learn:mainfrom
santiviquez:contingency_matrix_read_more
Feb 24, 2025
Merged

DOC add read more tagline for contingency matrix metric#30666
ArturoAmorQ merged 4 commits intoscikit-learn:mainfrom
santiviquez:contingency_matrix_read_more

Conversation

@santiviquez
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

What does this implement/fix? Explain your changes.

This PR adds the read more tagline for Contingency Matrix metric page. Which is currently missing.

Any other comments?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 17, 2025

✔️ Linting Passed

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

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

Copy link
Copy Markdown
Member

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

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

Hello @santiviquez,

thanks for your PR. There are a few issues, but all in all adding this cross link is very valuable.

Hint: It is very advisable to have pre-commit installed in your environment as in point 10 of How to contribute in the development guide. This will help with future linting issues.

While being at this PR, can you please also correct the typo in line 118 ("continency matrix")? Thanks.

Comment on lines +101 to +102

Read more in the :ref:`User Guide <contingency-matrix>`.
Copy link
Copy Markdown
Member

@StefanieSenger StefanieSenger Jan 19, 2025

Choose a reason for hiding this comment

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

Suggested change
Read more in the :ref:`User Guide <contingency-matrix>`.
Read more in the :ref:`User Guide <contingency_matrix>`.

This should fix the linting issue (which is a mean one, because it's about the indent of the blank line).

Also, I have corrected the link.

@ArturoAmorQ
Copy link
Copy Markdown
Member

Hi @santiviquez are you still working on this PR?
Do you need help addressing @StefanieSenger's comments?

@santiviquez
Copy link
Copy Markdown
Contributor Author

Hi @ArturoAmorQ, sorry I forgot about this. I'll take care of it today.

@santiviquez
Copy link
Copy Markdown
Contributor Author

Everything should look better now. Thanks for the suggestions @StefanieSenger, and for the reminder @ArturoAmorQ :)

Copy link
Copy Markdown
Member

@StefanieSenger StefanieSenger 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 further work, @santiviquez!

It looks very good now. :)

Copy link
Copy Markdown
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Thanks @santiviquez this is a nice improvement. Merging!

@ArturoAmorQ ArturoAmorQ merged commit 98af964 into scikit-learn:main Feb 24, 2025
33 checks passed
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