Skip to content
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.

Sign up for GitHub

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

Jump to bottom

DOC: add dendrogram example #2615

Merged
merged 9 commits into from
Dec 27, 2022
Merged

DOC: add dendrogram example #2615

merged 9 commits into from
Dec 27, 2022

Conversation

xujiboy
Copy link
Contributor

@xujiboy xujiboy commented Jun 11, 2022

I added an example to plot dendrogram following the example shown here. Please review when you have time. Thanks.

joelostblom
joelostblom requested changes Jun 12, 2022
View reviewed changes
Copy link
Contributor

@joelostblom joelostblom 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 submitting this PR! It looks like an interesting example to have in the gallery. I added a few comments that I think could help clarify the code.

altair/examples/dendrogram.py Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Show resolved Hide resolved
@xujiboy
Copy link
Contributor Author

xujiboy commented Jun 12, 2022

Thanks for the prompt review! I made some updates in the commit accordingly.

joelostblom
joelostblom reviewed Jun 13, 2022
View reviewed changes
Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

Looks great, just a few minor comments!

altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
altair/examples/dendrogram.py Outdated Show resolved Hide resolved
joelostblom
joelostblom approved these changes Jun 14, 2022
View reviewed changes
Copy link
Contributor

@joelostblom joelostblom 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 the updates! This looks good to me!

@xujiboy
Copy link
Contributor Author

xujiboy commented Jun 14, 2022

Thanks for the detailed review!

xujiboy and others added 9 commits December 27, 2022 22:53
@xujiboy @mattijn
DOC: add dendrogram example
8281fa1
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
cf76a12 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
4b44e4f 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
f96b902 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @mattijn
encoding code portion refactor and add comments to explain where the …
1aac620 
…input data comes from
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
016596f 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
ed2a02a 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @joelostblom @mattijn
Update altair/examples/dendrogram.py
9f682f9 
Co-authored-by: Joel Ostblom <[email protected]>
@xujiboy @mattijn
update comments to link the doc for scipy.cluster.hierarchy.dendrogram
4d726fd
@mattijn mattijn merged commit 0e6f7f1 into vega:master Dec 27, 2022
@mattijn
Copy link
Contributor

mattijn commented Dec 27, 2022

Rebased to retrigger the Github Actions. All pass now, so merging. Thanks again @xujiboy!

@xujiboy
Copy link
Contributor Author

xujiboy commented Dec 27, 2022

Thanks @mattijn

@xujiboy xujiboy deleted the example-dendrogram branch December 27, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelostblom joelostblom joelostblom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xujiboy @mattijn @joelostblom