Skip to content

DOC Add links to example plot_kmeans_stability_low_dim_dense.py#30349

Merged
adrinjalali merged 5 commits intoscikit-learn:mainfrom
yuanx749:plot_kmeans_stability_low_dim_dense
Dec 9, 2024
Merged

DOC Add links to example plot_kmeans_stability_low_dim_dense.py#30349
adrinjalali merged 5 commits intoscikit-learn:mainfrom
yuanx749:plot_kmeans_stability_low_dim_dense

Conversation

@yuanx749
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Towards #26927.

What does this implement/fix? Explain your changes.

This PR add links to example plot_kmeans_stability_low_dim_dense.py, which has not been included in either user guides or API docs.

Any other comments?

@yuanx749 yuanx749 changed the title Add links to example plot_kmeans_stability_low_dim_dense.py DOC Add links to example plot_kmeans_stability_low_dim_dense.py Nov 26, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 26, 2024

✔️ Linting Passed

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

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

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.

Hi @yuanx749,

thanks for your PR. I think the links are in the right spots for the docstrings. In the Userguide I would prefer to add those links only where it matters content-wise. We always take care the guide doesn't become too crowded.

I have left you some comments.

Comment on lines +247 to +248
* :ref:`sphx_glr_auto_examples_cluster_plot_kmeans_stability_low_dim_dense.py`:
Evaluating the impact of k-means initializations to convergence.
Copy link
Copy Markdown
Member

@StefanieSenger StefanieSenger Nov 26, 2024

Choose a reason for hiding this comment

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

Rather than adding this here without context, I would prefer to add this above in li. 227, after

"For a detailed example of comaparing different initialization schemes, refer to :ref:sphx_glr_auto_examples_cluster_plot_kmeans_digits.py." that already mentions a similar example. You could simple add your example in the same sentence with "and".

(Would be nice if you -while being there - could also correct the spelling mistake in comaparing. Thank you!)

yuanx749 and others added 3 commits November 26, 2024 20:38
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
@yuanx749
Copy link
Copy Markdown
Contributor Author

Thank you for the comments. @StefanieSenger I agree and have made the changes. I will keep these in mind in my future PRs. :)

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.

Awesome, @yuanx749. This looks very good now.

@adrinjalali, would you merge it?

@adrinjalali adrinjalali merged commit 5676cc5 into scikit-learn:main Dec 9, 2024
stefanogaspari pushed a commit to stefanogaspari/scikit-learn that referenced this pull request Dec 9, 2024
…ikit-learn#30349)

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
virchan pushed a commit to virchan/scikit-learn that referenced this pull request Dec 9, 2024
…ikit-learn#30349)

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
@yuanx749 yuanx749 deleted the plot_kmeans_stability_low_dim_dense branch December 13, 2024 06:01
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jan 8, 2025
…ikit-learn#30349)

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
jeremiedbb pushed a commit that referenced this pull request Jan 9, 2025
…0349)

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
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