Skip to content

Navigation Menu

Sign in
Appearance settings

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 plot_ols_ridge_variance example to the doc #30683

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 9 commits into from
Feb 19, 2025

Conversation

sotagg
Copy link
Contributor

@sotagg sotagg commented Jan 20, 2025

Reference Issues/PRs

#30621

What does this implement/fix? Explain your changes.

This PR adds a reference to the plot_ols_ridge_variance.py example in the Ridge Regression section of the User Guide (linear_model.rst). This example demonstrates how Ridge regression reduces variance compared to OLS, making it relevant to this section.

Copy link

github-actions bot commented Jan 20, 2025

✔️ Linting Passed

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

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

@StefanieSenger
Copy link
Contributor

Thanks for your contribution, @sotagg!

A concern that I would have is that the example on regression is listed below a part that treats classification. Two of the other examples listed there are also not dealing with RidgeClassifier.

What would you think of moving these three up into line 146 and making a new example section there?

@sotagg
Copy link
Contributor Author

sotagg commented Jan 21, 2025

Thank you for the feedback, @StefanieSenger!

I agree with your concern and have updated the documentation accordingly.
Let me know if further adjustments are needed!

Copy link
Contributor

@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.

That looks very good. Thank you @sotagg!

@marenwestermann @adrinjalali, would you like to merge this?

@adrinjalali
Copy link
Member

@sotagg thanks for the PR. As adding links to examples go, this is fine, but the example itself really needs some improvements.

I think we can merge this example with plot_ols.py and improve the description of what's happening in it. Would you be up for it in the same pull request?

@sotagg
Copy link
Contributor Author

sotagg commented Jan 22, 2025

Thanks for the suggestion, @adrinjalali!
I'll work on merging the examples and update the PR shortly.

@sotagg
Copy link
Contributor Author

sotagg commented Jan 25, 2025

@adrinjalali
I've merged the two files into a single example.
However, I want to get your thoughts on the design. Since this example fits under both the OLS and Ridge sections, would it make sense to duplicate the link in both places in the docs, or should we focus on one section and cross-reference the other?

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

You also need to remove the plot_ols_ridge_variance.py file (you can do with git rm)

And then, yes, it makes sense to reference this from ridge as well probably.

Copy link
Member

Choose a reason for hiding this comment

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

you can remove this file now

@sotagg
Copy link
Contributor Author

sotagg commented Feb 13, 2025

@adrinjalali
I’ve removed the two files and updated the document where the image generated from plot_ols.py is referenced.

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Nice, this was missing redirects from the old examples, which I've added.

@adrinjalali adrinjalali enabled auto-merge (squash) February 19, 2025 08:49
@adrinjalali adrinjalali merged commit ee4e163 into scikit-learn:main Feb 19, 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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.