R devel - May 2015 - Probably a "bug" in the dendextend package

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: www.talgalili.com (Hebrew) | www.biostatistics.co.il (Hebrew) |
www.r-statistics.com (English)
----------------------------------------------------------------------------------------------

	[[alternative HTML version deleted]]