Skip to content

DOC merge plot_ward_structured_vs_unstructured and plot_agglomerative_clustering#30861

Merged
marenwestermann merged 17 commits intoscikit-learn:mainfrom
mahmadza:add-example-link-plot_ward_structured_vs_unstructured
Sep 8, 2025
Merged

DOC merge plot_ward_structured_vs_unstructured and plot_agglomerative_clustering#30861
marenwestermann merged 17 commits intoscikit-learn:mainfrom
mahmadza:add-example-link-plot_ward_structured_vs_unstructured

Conversation

@mahmadza
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Towards #30621

What does this implement/fix? Explain your changes.

Added a link to structured vs unstructured ward hierarchical clustering example in AgglomerativeClustering class.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 19, 2025

✔️ Linting Passed

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

Generated for commit: 7b4f426. 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.

That looks good, thank you @mahmadza.

Basically, the examples are so similar, that we might want to join them later. But for now, I would merge this PR, since it is better to hint users to both usage examples than to only one, even if they don't differ so much.

@adrinjalali, what do you think about this?

@adrinjalali
Copy link
Copy Markdown
Member

They're almost the same example. I'd merge them in this PR. Would you be up for it @mahmadza ? You'd need to remove plot_agglomerative_clustering.py, and merge the relevant parts of its content with the other example. Then you'd add a redirect from the old example to the new one here:

redirects = {

And remove all mentions of the old example from the codebase.

@mahmadza
Copy link
Copy Markdown
Contributor Author

Sure I can do that. To summarize what I need to do:

  • merge relevant parts of examples.cluster.plot_agglomerative_clustering.py into examples.cluster.plot_ward_structured_vs_unstructured.py
  • delete examples.cluster.plot_agglomerative_clustering.py
  • add redirects from examples.cluster.plot_agglomerative_clustering.py to the new examples.cluster.plot_ward_structured_vs_unstructured.py in
    redirects = {
    .
  • delete all mentions of :ref:`sphx_glr_auto_examples_cluster_plot_agglomerative_clustering.py` in the code base.

Let me know if my understanding is correct.

@mahmadza
Copy link
Copy Markdown
Contributor Author

@StefanieSenger @adrinjalali All requested changes have been completed.

@marenwestermann
Copy link
Copy Markdown
Member

Hi @mahmadza, would you like to continue working on this pull request?

@mahmadza
Copy link
Copy Markdown
Contributor Author

mahmadza commented Jun 2, 2025

Hi @mahmadza, would you like to continue working on this pull request?

@adrinjalali @marenwestermann sorry for the delay.
@marenwestermann , you can take over if interested.

@marenwestermann
Copy link
Copy Markdown
Member

I combined the examples. However, the figures from the former example "Agglomerative clustering with and without structure" still need to be combined into one figure so that the rendering in the user guide is correct. I will do that in the coming days.

@marenwestermann
Copy link
Copy Markdown
Member

I updated this PR and addressed all the remaining comments. @adrinjalali can you give this one more review? I would like a second opinion before merging.

Copy link
Copy Markdown
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.

Otherwise LGTM.

@adrinjalali adrinjalali changed the title DOC add link to cluster_plot_ward_structured_vs_unstructured in _aggl… DOC merge plot_ward_structured_vs_unstructured and plot_agglomerative_clustering Sep 8, 2025
@marenwestermann marenwestermann enabled auto-merge (squash) September 8, 2025 17:26
@marenwestermann marenwestermann merged commit eabfce9 into scikit-learn:main Sep 8, 2025
36 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.

4 participants