Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

DOC Add "See Also" reference to ValidationCurveDisplay in validation_curve docstring #31314

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
merged 5 commits into from
May 15, 2025

Conversation

TheAyos
Copy link
Contributor

@TheAyos TheAyos commented May 5, 2025

Reference Issues/PRs

Aims to solve PR #31302.

What does this implement/fix? Explain your changes.

Updated the docstring of validation_curve, added "See Also" references to ValidationCurveDisplay.

Any other comments?

N/A.

Copy link

github-actions bot commented May 5, 2025

✔️ Linting Passed

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

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

@TheAyos TheAyos marked this pull request as ready for review May 5, 2025 20:21
@ogrisel
Copy link
Member

ogrisel commented May 6, 2025

@TheAyos this PR includes unrelated changes. Can you please update it to only include the change in the docstring?

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

In addition to the unrelated changes, here is some feedback to fix the problems found by looking at the CI report.

First clicking on the failures, in those reports to reach the logs you can see the following error message:

E            - GL07: Sections are in the wrong order. Correct order is: Parameters, Returns, See Also, Notes, Examples

Please put the "See also" section after the "Returns" section to fix the failing tests.

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved
sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved
Copy link
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 @TheAyos LGTM. I let @ogrisel do the merge as he was the OP of the issue.

@ArturoAmorQ
Copy link
Member

Well, I guess we can safely merge and iterate if needed.
Thanks again for your work and patience @TheAyos!

@ArturoAmorQ ArturoAmorQ merged commit 675736a into scikit-learn:main May 15, 2025
36 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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.