[Rd] Probably a "bug" in the dendextend package

[Martin Maechler]

>>>>> 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