search

talgalili / dendextend

Extending R's Dendrogram Functionality
152 stars 28 forks source link

Update get_subdendrograms.R #105

Closed chasemc closed 3 years ago

chasemc commented 3 years ago

get_subdendrograms() was passing the numeric index of the labels to find_dendrogram() instead of the actual labels. Fixes talgalili#104

talgalili commented 3 years ago

Thanks! I'll take a look within a week or so.

On Sun, Apr 11, 2021, 15:49 Chase Clark @.***> wrote:

get_subdendrograms() was passing the numeric index of the labels to find_dendrogram() instead of the actual labels. Fixes #104 https://github.com/talgalili/dendextend/issues/104

You can view, comment on, or merge this pull request online at:

https://github.com/talgalili/dendextend/pull/105 Commit Summary

  • Update get_subdendrograms.R

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/talgalili/dendextend/pull/105, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHOJBUFSCIKZXSYUY4SGWDTIGLGTANCNFSM42XUCKCA .

chasemc commented 3 years ago

There's some interesting notes on finding problems in R packages within this. Do you mind if I turn the problem solving I did into a short blog post?

talgalili commented 3 years ago

With pleasure. If you could also add some testthat functions, that would be great!

On Sun, Apr 11, 2021, 18:07 Chase Clark @.***> wrote:

There's some interesting notes on finding problems in R packages within this. Do you mind if I turn the problem solving I did into a short blog post?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/talgalili/dendextend/pull/105#issuecomment-817322762, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHOJBVRZ76WPVCH4CNSRADTIG3KLANCNFSM42XUCKCA .

talgalili commented 3 years ago

Thanks for the PR @chasemc !

I've now merged your code/fix (and bumped the version to 0.14.1). And also made another "fix" (since the output didn't have a good cluster order). See what I did here: https://github.com/talgalili/dendextend/commit/e4e303219a94840d2ad3c02d8be14701565b6bd4

In general, if you have other suggestions for things to improve/fix - please go ahead and do so! :)

I'll probably not upload this version to CRAN yet, will probably do it once I get to deal with some of the other issues people opened.