Skip to content

DOC plot classification probability #29921

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 28 commits into from
Oct 17, 2024
Merged

DOC plot classification probability #29921

merged 28 commits into from
Oct 17, 2024

Conversation

Sean-Jay-M
Copy link
Contributor

Reference Issues/PRs

issue #26927

What does this implement/fix? Explain your changes.

This PR links the plot_classification_probability example to the following classes:

  1. GaussianProcessClassifier
  2. Logistic Regression
  3. SVC

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 23, 2024 •
edited
Loading

✔️ Linting Passed

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

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

@Sean-Jay-M Sean-Jay-M mentioned this pull request Sep 23, 2024
Closed
@Sean-Jay-M Sean-Jay-M changed the title Doc plot classification probability DOC plot classification probability Sep 23, 2024
marenwestermann
marenwestermann reviewed Sep 28, 2024
View reviewed changes
Copy link
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.

Hi @Sean-Jay-M! Thank you for your PR. It generally looks good, it would just be better to move the references and shorten them a bit. Could you also check if the examples are listed in the respective "Examples" sections in the user guide (e.g. see here)? You can find the files for the user guide under sklearn/doc/modules.

@Sean-Jay-M
Copy link
Contributor Author

Sean-Jay-M commented Sep 30, 2024 •
edited
Loading

Hi @Sean-Jay-M! Thank you for your PR. It generally looks good, it would just be better to move the references and shorten them a bit. Could you also check if the examples are listed in the respective "Examples" sections in the user guide (e.g. see here)? You can find the files for the user guide under sklearn/doc/modules.

I have imeplemented the changes into the doc/modules and added the links there. Thank you for the directions ! Any other issues, let me know.

marenwestermann
marenwestermann reviewed Oct 1, 2024
View reviewed changes
Copy link
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.

Just one more comment. Otherwise it looks good

marenwestermann
marenwestermann approved these changes Oct 3, 2024
View reviewed changes
Copy link
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.

LGTM

ping @adrinjalali

@Sean-Jay-M
Copy link
Contributor Author

@adrinjalali Please review :)

@adrinjalali
Copy link
Member

I removed two instances since we want to keep them mostly relevant.

@adrinjalali adrinjalali enabled auto-merge (squash) October 17, 2024 12:24
glemaitre
glemaitre approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Approving on behalf of @adrinjalali

@adrinjalali adrinjalali merged commit d2acd79 into scikit-learn:main Oct 17, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre approved these changes

@marenwestermann marenwestermann marenwestermann approved these changes

Assignees
No one assigned
Labels
Documentation module:gaussian_process module:linear_model module:svm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Sean-Jay-M @adrinjalali @glemaitre @marenwestermann