>>>>> Tal Galili <tal.gal...@gmail.com> >>>>> on Mon, 18 May 2015 23:01:44 +0300 writes:
> The problem: > =========== > Once a dendrogram has a branch with both a line type AND a color (which is > a character color), the plot.dendrogram function will not plot and return > an error. If the dendrogram has been messed up ... see below. > I say this is a bug because (I believe), we would like a dendrogram to be > able to use character colors, while also allowing control over line types. I say this is clearly a bug in the code that messed up the dendrogram, see below. > This e-mail includes an example, and what I think a solution might be. > Reproducible example: > ================= > install.packages('dendextend') > library('dendextend') > dend <- 1:2 %>% dist %>% hclust %>% as.dendrogram > plot(dend) # works fine > dend %>% set("branches_lty", 1:2) %>% plot # works fine > dend %>% set("branches_col", 1:2) %>% plot # works fine > dend %>% set("branches_col", as.character(1:2)) %>% plot # works fine > # Fails: > dend %>% set("branches_lty", 1:2) %>% set("branches_col", > as.character(1:2)) %>% plot > ### Error: > # Error in segments(x0, y0, x1, y1, col = col, lty = lty, lwd = lwd) : > # invalid line type: must be length 2, 4, 6 or 8 Well, the above magrittr pipe mumbo-jumbo does not make it easy to deparse what you are doing ... and below you see why I think the bug is only in your 'dendextend' package. Please -- for supposed bugs in R, we do need reproducible examples *not* making use of external packages ! > # This is because edgePar has to hold both "lty" and "col" > # Since "col" is a character, it forces "lty" to become a character also. This is nonsense, sorry: If you'd read the help for plot.dendrogram(), edgePar is clearly defined as list, not vector, so it can well contain character and integer vectors.... and that design (list, not atomic vector) was exactly for this reason. I'd tend to say the bug *is* in your package only: when your set() function treats edgePar as atomic vector instead of list ??? > dend %>% set("branches_lty", 1:2) %>% set("branches_col", > as.character(1:2)) %>% > unclass %>% str again the pipe mumbo-jumbo .. I hope a version of dendextend::set() would allow several arguments, so the above would back translate to proper functional (*and* more efficient!) language d2 <- set(dend, branches_lty = 1:2, branches_col = c("1","2")) str(unclass(d2)) [... what should col = "1" mean ?? a more convincing example would use col = c("tomato", "orange") (using ``eatable colors'' just for fun) [ Currently, I don't see a bug in R's dendrogram code but rather in your package. > Possible solution > ============== > The simplest (and backward) compatible solution I can think of is to edit > the function: > stats:::plotNode > And change the following lines: > col <- Xtract("col", ePar, default = par("col"), > i) > lty <- Xtract("lty", ePar, default = par("lty"), > i) > lwd <- Xtract("lwd", ePar, default = par("lwd"), > i) > With: > col <- Xtract("col", ePar, default = par("col"), > i) > lty <- as.numeric(Xtract("lty", ePar, default = par("lty"), > i)) > lwd <- as.numeric(Xtract("lwd", ePar, default = par("lwd"), > i)) This would break cases where you'd make use of the feature that lty in R has always been much more flexible than just taking the few predefined integer line types: Read ?par and grep for the several occurences of 'lty'.. > With regards, > Tal > [[alternative HTML version deleted]] ^^^^^^^^^^ hmm... ... after all that: Your dendextend *is* a nice package, Tal, and so "best regards!", but as I've said before on this topic: In real life, be very careful before denigrating a child to its mother or father... so calling a something a bug in R which is none, is evoking feelings among R's parents .. ;-) Martin ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel