Skip to content

DOC add plot_ols_ridge_variance example to the doc#30683

Merged
adrinjalali merged 9 commits intoscikit-learn:mainfrom
sotagg:doc/plot_ols_ridge_variance
Feb 19, 2025
Merged

DOC add plot_ols_ridge_variance example to the doc#30683
adrinjalali merged 9 commits intoscikit-learn:mainfrom
sotagg:doc/plot_ols_ridge_variance

Conversation

@sotagg
Copy link
Copy Markdown
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.

@github-actions
Copy link
Copy Markdown

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
Copy Markdown
Member

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
Copy Markdown
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
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.

That looks very good. Thank you @sotagg!

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

@adrinjalali
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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