waldronlab / MultiAssayExperiment

Bioconductor package for management of multi-assay data
https://waldronlab.io/MultiAssayExperiment/
70 stars 32 forks source link

Constructing `MatchedAssayExperiment` from `ExperimentList` #267

Closed cvanderaa closed 4 years ago

cvanderaa commented 4 years ago

Hi, I'm trying to construct a MatchedAssayExperiment supplying only an ExperimentList object under the experiment parameter. Here is some code to reproduce my issue:

library(MultiAssayExperiment)

# Create a mock ExperimentList
assayMatrix <- matrix(rnorm(100), 10, 10, 
                      dimnames = list(1:10, letters[1:10]))
x <- lapply(1:3, function(i) SummarizedExperiment(assays = assayMatrix))
names(x) <- LETTERS[1:3]
x <- do.call(ExperimentList, x)

## This works nicely
mae1 <- MultiAssayExperiment(experiments = x)
mae1

## Issue is here 
mae2 <- MatchedAssayExperiment(experiments = x)
## Error in MatchedAssayExperiment(experiments = x) : 
##   Provide a 'MultiAssayExperiment' or its basic components

## However, this works nicely
mae3 <- MatchedAssayExperiment(experiments = x, 
                               metadata = list("some metadata"))
mae3

Is this expected?

lwaldron commented 4 years ago

The issue is just that by providing the experiments argument, MatchedAssayExperiment() is expecting an ExperimentList. Try this:

mae2 <- MatchedAssayExperiment(mae1)
cvanderaa commented 4 years ago

Indeed this works. Thank you ! Maybe the MatchedAssayExperiment documentation could be slightly adapted to avoid this confusion? But that's only a minor comment, I close the issue.

lwaldron commented 4 years ago

Marcel, @LiNk-NY, the confusion here arose from this length check:

https://github.com/waldronlab/MultiAssayExperiment/blob/524707e0b2908ddc4371cc00257755e900cf7eab/R/MultiAssayExperiment-class.R#L688

which specifies that a single argument must be a MultiAssayExperiment argument, but multiple arguments are arguments to the MultiAssayExperiment() constructor. I wonder if this check creates more confusion than good - without out it, you would just have:

MatchedAssayExperiment <- function(...) {
    listData <- list(...)
        if (is(listData[[1L]], "MultiAssayExperiment")){
            multiassay <- listData[[1L]]
       } else {
           multiassay <- MultiAssayExperiment(...)
       }
    multiassay <- .doMatching(multiassay)
    new("MatchedAssayExperiment", multiassay)
}

Then you would rely on MultiAssayExperiment() to return an informative error message, or you could add a try-catch to create something more specific. ?MatchedAssayExperiment does not explicitly say that ... are passed on to MultiAssayExperiment() if more than one argument is given. Re-opening for now.