Skip to content

DOC: Add seealso link to Ridge regression example in user guide (#30621) #31581

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
Closed
wants to merge 4 commits into from
Closed

DOC: Add seealso link to Ridge regression example in user guide (#30621) #31581

wants to merge 4 commits into from

Conversation

dhyeyinf
Copy link
Contributor

@dhyeyinf dhyeyinf commented Jun 18, 2025 •
edited
Loading

This PR adds a contextual seealso reference in the Ridge regression section of the user guide to link the plot_ridge_path.py example.

Although the example was already listed at the bottom, this inline reference improves discoverability for readers following the theoretical explanation.

Towards #30621 for this example.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 18, 2025 •
edited
Loading

✔️ Linting Passed

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

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

adrinjalali
adrinjalali reviewed Jun 18, 2025
View reviewed changes
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.

The user guide already has enough links to that example. You could instead add a link to the example in the docstring of alpha in Ridge.

doc/modules/linear_model.rst Outdated
Comment on lines 112 to 117
.. seealso::

For a visual demonstration of how Ridge coefficients evolve with regularization,
see *plot_ridge_path.py*:
:ref:`sphx_glr_auto_examples_linear_model_plot_ridge_path.py`

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we use the .. seealso directive for these. But maybe we should.

@dhyeyinf
Copy link
Contributor Author

Hi!
Thanks for the helpful feedback. I've removed the user guide change and instead updated the alpha parameter docstring in the Ridge class to include a link to the relevant example (plot_ridge_path.py), as suggested.

Let me know if any further adjustments are needed. Thanks again!

adrinjalali
adrinjalali reviewed Jun 19, 2025
View reviewed changes
doc/modules/linear_model.rst Outdated
@@ -109,6 +109,13 @@ The complexity parameter :math:`\alpha \geq 0` controls the amount
of shrinkage: the larger the value of :math:`\alpha`, the greater the amount
of shrinkage and thus the coefficients become more robust to collinearity.

.. seealso::
Copy link
Member

Choose a reason for hiding this comment

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

I think you forgot to remove these or push the removal here.

@dhyeyinf
Copy link
Contributor Author

Hi @adrinjalali,
Thanks for pointing that out — I just noticed the leftover seealso block hadn’t been removed properly. I’ve now removed it from linear_model.rst and pushed the changes. Apologies for the oversight earlier, and thank you again for the review and guidance!

@StefanieSenger StefanieSenger mentioned this pull request Jun 24, 2025
Open
@StefanieSenger
Copy link
Contributor

Hi @dhyeyinf, thanks for your contribution!

Would you mind to also change the Closes #... in your issue description into something like Towards #...? Otherwise you are using a command to close the whole issue when your PR gets merged. Thank you!

@dhyeyinf
Copy link
Contributor Author

Updated the PR description to say "Towards #30621" instead of "Closes #30621" — thanks for the reminder!

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Jun 25, 2025 •
edited
Loading

Thanks @dhyeyinf, could you please explain and restore the change in the doc/modules/linear_model.rst file? If it is a tabs versus spaces thing, then our linter should actually fail.

@dhyeyinf
Copy link
Contributor Author

Thanks for the follow-up @adrinjalali! I've restored the inline example reference above the figure in linear_model.rst. Appreciate your patience and guidance — please let me know if any other changes are needed.

@StefanieSenger
Copy link
Contributor

Hi @dhyeyinf, it seems your AI tool didn't understand my request to restore your change in linear_model.rst at all.

Please respect our Automated Contributions Policy.

I will therefore close this PR.

@dhyeyinf
Copy link
Contributor Author

Hi @adrinjalali,
Thanks for the review and your patience. I now understand I misinterpreted your request about restoring the example link in linear_model.rst. I appreciate the feedback and take full responsibility.

This has been a valuable learning experience, and I’ll be more careful and precise in future contributions. Looking forward to contributing again with a better approach.

Best,
Dhyey

@StefanieSenger StefanieSenger mentioned this pull request Jun 26, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali left review comments

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhyeyinf @StefanieSenger @adrinjalali