Skip to content

DOC Add example links to LabelSpreading class docstring and update example headings #30553

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ani0075saha
Copy link
Contributor

Reference Issues/PRs

PR to add links to the examples mentioned in issue #26927 for the semi_supervised section.

What does this implement/fix? Explain your changes.

Links to examples were added to the API docs of LabelSpreading class.

plot_label_propagation_digits_active_learning.py example contains the functionality described in plot_label_propagation_digits.py.

So I only added links to

plot_label_propagation_digits_active_learning.py
plot_label_propagation_structure.py

I also modified the example headings.

Any other comments?

Copy link

github-actions bot commented Dec 28, 2024

✔️ Linting Passed

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

Generated for commit: 214192c. Link to the linter CI: here

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Jan 10, 2025

Hello @ani0075saha, thank you for your contribution.

The new headings look much friendlier.

Concerning the links, I would actually tend to not link the examples in LabelSpreading, because they're not very specific and clicking on the link to the User Guide just one line below, brings us to a short section that already references the examples.

Would you mind to remove the links from the docstring and only keep the rest of the PR (the headings)?

That would be alright as is - or - if you like, you could also work further on improving the examples on LabelSpreading, because I see they are not in such a good shape. If you wonder what a good shape for an example looks like, you could look here (of cause it doesn't need to be as elaborate; something like this with code and text mixed would be definitely an improvement).

@ani0075saha
Copy link
Contributor Author

@StefanieSenger Thanks for the comments. I will look at the linked good quality examples and work on improving the LabelSpreading examples.

@marenwestermann
Copy link
Member

Hi @ani0075saha! Would you like to continue working on this PR?

@ani0075saha
Copy link
Contributor Author

Hi @marenwestermann, it seems like I won't have the bandwidth right now to make the examples better as @StefanieSenger suggested. We can merge and close this right now with just the updated headings or someone else can continue the work. Let me know what's the best option.

@StefanieSenger
Copy link
Contributor

Hi @marenwestermann, it seems like I won't have the bandwidth right now to make the examples better as @StefanieSenger suggested. We can merge and close this right now with just the updated headings or someone else can continue the work. Let me know what's the best option.

Hello @ani0075saha, that's totally fine. Then I would suggest to revert the changes in sklearn/semi_supervised/_label_propagation.py and let this be a PR that improves the headings, which is valuable per se, since the headings had been quite odd before. What do you think, @marenwestermann?

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.

Thanks for improving the headings, @ani0075saha!
Your contribution is very welcome.

@adrinjalali, would you mind having a look?

@ani0075saha
Copy link
Contributor Author

Sorry about the long delay @StefanieSenger. Ran into some git issues today, but resolved them finally.

@ani0075saha
Copy link
Contributor Author

@adrinjalali Can you please review and close this? Thanks!

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