assayNames<- is reluctant to work about summarizedexperiment HOT 5 CLOSED

Done in SummarizedExperiment 1.15.5 (see commit 7df70dc).

@LTLA This addresses your original issue so I'm closing this. Feel free to re-open if I missed something.

from summarizedexperiment.

Hi Aaron,

The dimnames setter for SE objects is at the root of the problem.

My understanding is that the inner dimnames (i.e. the dimnames on the individual assays) are only allowed to be NULL or the same as the outer dimnames (i.e. the dimnames of the SE object itself). This is actually enforced by the SE constructor:

library(SummarizedExperiment)
m <- matrix(1:12, nrow=4)
rowData <- DataFrame(stuff=11:14, row.names=letters[1:4])

se1 <- SummarizedExperiment(m, rowData=rowData)
dimnames(se1)
# [[1]]
# [1] "a" "b" "c" "d"
#
# [[2]]
# NULL

colnames(m) <- LETTERS[1:3]
se2 <- SummarizedExperiment(m, rowData=rowData)
dimnames(se2)
# [[1]]
# [1] "a" "b" "c" "d"
#
# [[2]]
# [1] "A" "B" "C"

rownames(m) <- LETTERS[1:4]
se3 <- SummarizedExperiment(m, rowData=rowData)
# Error in .validate_names(rownames, ans_rownames, "assay rownames()", "rowData rownames() / rowRanges names()") : 
#   assay rownames() must be NULL or identical to rowData rownames() /
#   rowRanges names()

Unfortunately, for some reason, this is not checked by the validity method for SummarizedExperiment objects.

Now when you do:

mat <- matrix(1,1,1, dimnames=list("B", "GENE2"))
se <- SummarizedExperiment(list(counts=mat))
dimnames(se) <- list("A", "GENE1")

you end up with an object that is "wrong" (even though validObject(se) returns TRUE):

dimnames(se)
# [[1]]
# [1] "GENE1"
#
# [[2]]
# [1] "A"

dimnames(assay(se, withDimnames=FALSE))
# [[1]]
# [1] "GENE2"
#
# [[2]]
# [1] "B"

validObject(se)
# [1] TRUE

This "wrong" object causes problems later when you do assayNames(se) <- "whee" or simply do the following no-op:

assays(se, withDimnames=FALSE) <- assays(se, withDimnames=FALSE)
# Error in `assays<-`(`*tmp*`, withDimnames = FALSE, value = new("SimpleList",  : 
#   current and replacement dimnames() differ

So one thing we can do is fix the dimnames setter by having it adjust (or drop) the inner dimnames to keep them in sync with the new outer dimnames.

@mtmorgan: But why do we care about the inner dimnames in the first place? Never really understood the motivation for this constraint. It feels arbitrary and counter productive to me. Couldn't we just ignore the inner dimnames? Benefits: simpler conceptual validity, simpler implementation of the SE constructor and a few setters (dimnames<-, assays<-), faster code (adjusting the inner dimnames could be expensive in some situations), and no more problems of the sort Aaron ran into. What could the downsides be?

If Martin has no objection, I'll remove this constraint.

from summarizedexperiment.

I think the inner dimnames are there for efficiency -- the most common way to access the data is assay(); having dimnames on the result is usually desired; it's expensive (causes the assay to be copied) to add them. This was motivated by a use case / problem Kasper Hansen had several years ago with large methylation arrays, I don't know whether his concerns are still relevant...

from summarizedexperiment.

Thanks Martin. Just to clarify: right now the inner dimnames can be NULL so we already are in a situation where they don't necessarily match the outer dimnames. And assay() is already triggering copies when setting the outer dimnames on the returned assay (unless withDimnames=FALSE is used). I'm not proposing to change that. All I'm proposing is that we let the inner dimnames be anything,

from summarizedexperiment.

ok by me.

from summarizedexperiment.