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
fixes dendrogram issue #1228 #1614
Conversation
Hi @fidelram ! |
Hi @giovp, basically I mimic the what the linkage code would do internally when the first argument is not a distance. Because the correlation matrix is used for |
Makes sense! Thanks it's clear now why the corr matrix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this get:
- Bugfix release note
- Links to issues that should be closed by this
Issues that should be closed: |
@meeseeksdev backport to 1.7.x |
@fidelram, in future, if you add a line like:
To the initial comment in a PR, those issues will automatically be closed when the PR is merged. I think you can also link PRs to issues with the "Linked issues" element in the righthand sidebar. Using these makes it easier to tell which issues have been addressed already. |
Co-authored-by: Fidel Ramirez <fidel.ramirez@gmail.com>
The PR uses
1-correlation
as distance matrix to compute the dendrogram as suggested in #1288 and mentioned in scverse/squidpy#236I opted for the minimal changes to the code. Other solution would be to use
scipy.spatial.distance.pdist
that allows a larger number of distance metrics. I am open for a discussion on this topic (ping @michalk8)