talgalili / dendextend

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

fixed 'type = "triangle"' error #99

Closed ghost closed 4 years ago

ghost commented 4 years ago

fixed #32

codecov-io commented 4 years ago

Codecov Report

Merging #99 into master will decrease coverage by 0.00%. The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
- Coverage   34.98%   34.97%   -0.01%     
==========================================
  Files          50       50              
  Lines        3733     3734       +1     
==========================================
  Hits         1306     1306              
- Misses       2427     2428       +1     
Impacted Files Coverage Δ
R/ggdend.R 88.67% <80.00%> (-0.85%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 49ad39c...6cb664f. Read the comment docs.

talgalili commented 4 years ago

Hi @houyunhuang Thanks for the PR. Could you please add an example or two (or even a basic unit test on a simple case) to the code, to help me check it? Thanks!

ghost commented 4 years ago

I'm sorry I didn't see this until today.

Here is a simple example:

library(ggplot2)
library(dendextend)
library(patchwork)
library(dplyr)
dend <- mtcars %>% dist %>% hclust %>% as.dendrogram %>%
   set("branches_k_color", k=3) %>% set("branches_lwd", c(1.5,1,1.5)) %>%
   set("branches_lty", c(1,1,3,1,1,2)) %>%
   set("nodes_pch", 19) %>% set("nodes_col", c("orange", "black", "plum", NA))
plot(dend, leaflab = "none")

ggd1 <- as.ggdend(dend, type = "triangle")
ggd2 <- as.ggdend(dend, type = "rectangle")
p1 <- ggplot(ggd1, labels = FALSE)
p2 <- ggplot(ggd2, labels = FALSE)
p1 / p2

Unfortunately, I just found that type parameter needs to be checked (match,arg) in the as.ggdend.dendrogram() function. But I'm not familiar with git, could you help me improve this pr?

talgalili commented 4 years ago

You just need to update the code and send another push. If you can't find out how to do it I can look at what you sent and solve it (but it would take me some time to get to it).

Thanks for the work :)

On Wed, Apr 15, 2020, 16:35 houyun notifications@github.com wrote:

I'm sorry I didn't see this until today.

Here is a simple example:

library(ggplot2)

library(dendextend)

library(patchwork)

library(dplyr) dend <- mtcars %>% dist %>% hclust %>% as.dendrogram %>%

set("branches_k_color", k=3) %>% set("branches_lwd", c(1.5,1,1.5)) %>%

set("branches_lty", c(1,1,3,1,1,2)) %>%

set("nodes_pch", 19) %>% set("nodes_col", c("orange", "black", "plum", NA))

plot(dend, leaflab = "none")

ggd1 <- as.ggdend(dend, type = "triangle") ggd2 <- as.ggdend(dend, type = "rectangle") p1 <- ggplot(ggd1, labels = FALSE) p2 <- ggplot(ggd2, labels = FALSE) p1 / p2

Unfortunately, I just found that type parameter needs to be checked ( match,arg) in the as.ggdend.dendrogram() function. But I'm not familiar with git, could you help me improve this pr?

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

talgalili commented 4 years ago

Hi @houyunhuang - any update?

ghost commented 4 years ago

@talgalili I'm really sorry that I forgot to submit pr. Now you can test whether this bug has been fixed or not.

talgalili commented 4 years ago

Thanks @houyunhuang It looks good from the checks I've done. I've merged your PR, and also added you as one of the contributors of the package (see DESCRIPTION). I'll wait to see if there are other issues (either from this PR, or other things people would ask) before pushing this to CRAN.

Please let me know if you plan any other additions to the package in the near future.

Cheers, T