8000 DOC add link plot_inductive_clustering by claudio1975 · Pull Request #30158 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC add link plot_inductive_clustering #30158

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

Closed

Conversation

claudio1975
Copy link
Contributor

What does this implement/fix? Explain your changes.

Added link plot_inductive_clustering -#26927

Copy link

✔️ Linting Passed

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

Generated for commit: 6876dd1. Link to the linter CI: here

Comment on lines -140 to -141
:term:`inductive` clustering methods) are not designed to be applied to new,
unseen data.
Copy link
Member

Choose a reason for hiding this comment

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

This marks the key difference between Transductive and Inductive Clustering. Therefore, I think it should be preseverd.

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 PR @claudio1975! I have some suggestions. Otherwise, LGTM.

Comment on lines 140 to 142
:term:`inductive` clustering methods, refer to this example
:ref:`sphx_glr_auto_examples_cluster_plot_inductive_clustering.py`) are not
designed to be applied to new, unseen data.
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 the example link should be placed under a subsection. You can scroll down the document a bit to see how it's done for k_means, which is rendered as:

k_means_doc_string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you too @virchan !!!
I agree, it's better to put the example outside the brackets. I've update the PR, and you can check it now -:)

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 updating the PR! I have a few more suggestions. Additionally, please ensure that the CI tests are addressed afterward. Once that’s done, I can notify the other reviewers.

Comment on lines +140 to +141
:term:`inductive` clustering methods) are not
designed to be applied to new, unseen data.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:term:`inductive` clustering methods) are not
designed to be applied to new, unseen data.
:term:`inductive` clustering methods) are not designed to be applied to new,
unseen 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.

Hi @virchan thanks for the suggestions.
I've seen yesterday that when I launched the updated PR, it started directly from the own repository instead from my fork repository.
I received the notifications about issues on the CI failed tests, and for instance I report the comment from the linter step:
""
Error: Unable to download artifact(s): Artifact not found for name: lint-log
Please ensure that your artifact is not expired and the artifact was uploaded using a compatible version of toolkit/upload-artifact.
""
Maybe could come from not updated files in the own repository?
To avoid troubles, I close the pull request, I'll update my fork and local repository, and then I'll open a new one.

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

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.

@claudio1975
Copy link
Contributor Author
claudio1975 commented Oct 31, 2024

will follow a new one -#30182

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.

2 participants
0