Tal Galili
2015-May-18 20:01 UTC
[Rd] A "bug" in plot.dendrogram - can't plot lty with character color
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. 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. 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 # This is because edgePar has to hold both "lty" and "col" # Since "col" is a character, it forces "lty" to become a character also. dend %>% set("branches_lty", 1:2) %>% set("branches_col", as.character(1:2)) %>% unclass %>% str 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)) With regards, Tal [[alternative HTML version deleted]]
>>>>> Tal Galili <tal.galili at 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
Dear Martin, You are right. When implementing the dendextend::set function, I failed to notice that edgePar should accept a list instead of a vector. So all I did was to discover a bug in my own code. I am both sorry for taking your time due to my own mistake, and also grateful for your help (I will resolve this bug before submitting the next release to CRAN). I am sure that I would have had similar feelings if the situation happened to me - so again, I apologize :) With regards, Tal On Wed, May 20, 2015 at 10:39 AM, Martin Maechler < maechler at lynne.stat.math.ethz.ch> wrote:> u----------------Contact Details:------------------------------------------------------- Contact me: Tal.Galili at gmail.com | Read me: talgalili.com (Hebrew) | biostatistics.co.il (Hebrew) | r-statistics.com (English) ---------------------------------------------------------------------------------------------- [[alternative HTML version deleted]]