Skip to content

DOC add link plot_inductive_clustering #30182

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 4 commits into from
Feb 16, 2025

Conversation

claudio1975
Copy link
Contributor

What does this implement/fix? Explain your changes.

Added link plot_inductive_clustering towards -#26927. It follows previous PR #30158

Copy link

github-actions bot commented Oct 31, 2024

✔️ Linting Passed

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

Generated for commit: 3b7e3c7. Link to the linter CI: here

@claudio1975
Copy link
Contributor Author

@virchan I've create a new PR and now everything seems on track

Copy link
Member

@virchan virchan left a comment

Choose a reason for hiding this comment

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

Thank you for the follow-up @claudio1975! I have one more suggestion.

Comment on lines 145 to 146
* :ref:`sphx_glr_auto_examples_cluster_plot_inductive_clustering.py`: A practical
example of inductive clustering model.
Copy link
Member

@virchan virchan Oct 31, 2024

Choose a reason for hiding this comment

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

Suggested change
* :ref:`sphx_glr_auto_examples_cluster_plot_inductive_clustering.py`: A practical
example of inductive clustering model.
* :ref:`sphx_glr_auto_examples_cluster_plot_inductive_clustering.py`: An example
of an inductive clustering model for handling new data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok it's fine for me @virchan to commit your suggetion, but before to do it, could you add the indentation to the second line, please?

Co-authored-by: Virgil Chan <virchan.math@gmail.com>
Copy link
Member

@virchan virchan left a comment

Choose a reason for hiding this comment

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

LGTM! I'd like to ping @marenwestermann for a quick look to help avoid any potential conflicts with PR #30127, as both involve changes to the doc/modules/clustering.rst file.

Also cc'ing @adrinjalali.

Copy link
Member

@marenwestermann marenwestermann left a comment

Choose a reason for hiding this comment

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

LGTM. But yes, let's wait until #30127 is merged. The changes can then be pulled into this branch which should cause a merge conflict. After resolving it, this PR can be merged.
I think it's ok to not link this example in the docstrings of AgglomerativeClustering and RandomForestClassifier because this example is about showing the principle of inductive clustering and not about the estimators.

@marenwestermann marenwestermann merged commit bccd697 into scikit-learn:main Feb 16, 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