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.

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

fixed 'type = "triangle"' error #99

Merged
merged 2 commits into from May 31, 2020
Merged

fixed 'type = "triangle"' error #99

merged 2 commits into from May 31, 2020

Conversation

ghost
Copy link

@ghost ghost commented Apr 7, 2020

fixed #32

@codecov-io
Copy link

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%) ⬇️

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
Copy link
Owner

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
Copy link
Author

ghost commented Apr 15, 2020

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
Copy link
Owner

talgalili commented Apr 15, 2020 via email

@talgalili
Copy link
Owner

Hi @houyunhuang - any update?

@ghost
Copy link
Author

ghost commented May 28, 2020

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

@talgalili talgalili merged commit 7043bde into talgalili:master May 31, 2020
@talgalili
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using as.ggdend() with type = "triangle"
3 participants