waldronlab / MultiAssayExperiment

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

Show Method Breaks if R Package tictoc is Loaded #321

Closed DarioS closed 1 year ago

DarioS commented 1 year ago

Could it be made robust against other packages breaking the display of the experiment list? It shows fine without tictoc loaded.

library(curatedTCGAData)
melanoma <- curatedTCGAData(diseaseCode = "SKCM", assays = c("RNASeq2GeneNorm", "miRNASeqGene"), 
                            version = "2.0.1", dry.run = FALSE)
melanoma
# A MultiAssayExperiment object of 2 listed
# experiments with user-defined names and respective classes.
# Containing an ExperimentList class object of length 2:
# [1] SKCM_miRNASeqGene-20160128: SummarizedExperiment with 1046 rows and 452 columns
# [2] SKCM_RNASeq2GeneNorm-20160128: SummarizedExperiment with 20501 rows and 473 columns
# Functionality:

But, loading tictoc prevents the correct display of the experiment list component. It is blank.

library(tictoc)
melanoma
# A MultiAssayExperiment object of 2 listed
# experiments with user-defined names and respective classes.
# Containing an ExperimentList class object of length 2:
# Functionality:
LiNk-NY commented 1 year ago

Hi Dario, @DarioS This is an issue for the tictoc maintainers not for MultiAssayExperiment. Best, Marcel

mtmorgan commented 1 year ago

Can you explain what's going on? It looks like there is tictoc::as.List, and this interferes somehow in vapply(), where the definition is

> vapply
function (X, FUN, FUN.VALUE, ..., USE.NAMES = TRUE)
{
    FUN <- match.fun(FUN)
    if (!is.vector(X) || is.object(X))
        X <- as.list(X)
    .Internal(vapply(X, FUN, FUN.VALUE, USE.NAMES))
}
<bytecode: 0x14a88c3f8>
<environment: namespace:base>

and things go wrong

> debug(vapply)
> experiments(melanoma)
debugging in: vapply(object, function(o) {
    class(o)[[1L]]
}, character(1L))
debug: {
    FUN <- match.fun(FUN)
    if (!is.vector(X) || is.object(X))
        X <- as.list(X)
    .Internal(vapply(X, FUN, FUN.VALUE, USE.NAMES))
}
Browse[2]> n
debug: FUN <- match.fun(FUN)
Browse[2]>
debug: if (!is.vector(X) || is.object(X)) X <- as.list(X)
Browse[2]>
debug: X <- as.list(X)
Browse[2]> class (X)
[1] "ExperimentList"
attr(,"package")
[1] "MultiAssayExperiment"
Browse[2]> n
debug: .Internal(vapply(X, FUN, FUN.VALUE, USE.NAMES))
Browse[2]> class(X)
[1] "list"
Browse[2]> length(X)
[1] 0

but why is as.list(X) selecting(?) the wrong method? And can't MultiAssayExperiment (and more generally S4Vectors?) protect against this?

mtmorgan commented 1 year ago

Simplifying ---

> vapply(S4Vectors::List("a", "b"), as.character, character(1))
[1] "a" "b"
> library(tictoc)
> vapply(S4Vectors::List("a", "b"), as.character, character(1))
character(0)

and I guess a little further there is

> tictoc:::as.list.List
function (x, ...)
as.list(x$.Data)
<bytecode: 0x1090205e8>
<environment: namespace:tictoc>

so in a new session there is

> vapply(S4Vectors::List("a", "b"), as.character, character(1))
[1] "a" "b"
> as.list.List <- function(x, ...) character()
> vapply(S4Vectors::List("a", "b"), as.character, character(1))
named character(0)
LiNk-NY commented 1 year ago

Thanks Martin @mtmorgan for looking into it.

> conflict_scout()
1 conflict:
* `List`: tictoc, S4Vectors

Is there a way to safeguard against the selection of the wrong method in this case? Or is it best to avoid tictoc?

mtmorgan commented 1 year ago

I'm not sure if there's a solution; maybe @hpages has a suggestion.

One could program around this in the show method in particular (e.g., by avoiding vapply() or coercing experiments() to a plain-old list via explicit calls to S4Vectors::as.list.List) but it seems like other parts of MultiAssayExperiment and S4Vectors are likely to be broken.

Maybe this is significant enough that one would want to do something with setHook() (in S4Vectors...) to warn the user that they are doing something incompatible, but that seems like a pretty funky thing to do...

bblodfon commented 1 year ago

Hi @LiNk-NY,

I have also noticed this issue recently (wasn't there 1 year ago) => it can happen even without loading anything else rather than curatedTCGAData, TCGAutils and dplyr! Also, sometimes running via Rscript or inside Rstudio (even as a background job) leads to different behavior in printing. I haven't been able to narrow it down, I thought it was worth mention it here.

hpages commented 1 year ago

I completely missed this, sorry.

Well, in that case we are lucky: List in S4Vectors is a virtual class, which means that the class of our objects can never be List. It's always a subclass of List e.g. SimpleList in the case of S4Vectors::List("a", "b"). So defining as.list.SimpleList addresses the issue, because it takes precedence over any as.list.List method.

The fix is in S4Vectors 0.37.5 (see https://github.com/Bioconductor/S4Vectors/commit/a0e7cdcc65ba73f1eda79281ba311d611b53c1ed). I also added a few other S3 as.list methods.