Constructor assigns wrong assay values to wrong samples and features about summarizedexperiment HOT 7 CLOSED

@TuomasBorman, @LiNk-NY,

I can think of 2 ways to deal with this:

1. The rows/cols of the supplied assay (or assays) get reordered to align with the rownames/colnames of the SE object (the rownames/colnames of the SE are inferred from the rownames of the rowData/colData).
2. We raise an error if one of the supplied assays has rownames and/or colnames that don't match those of the SE object.

The second option (2.) is consistent with the assay()/assays() setters so that's my preference. The first option (1.) is more complicated than it sounds e.g. it's not clear what to do if the rownames/colnames of the user-supplied assay are completely different from those of the SE object or if only a subset of them match those of the SE object.

I've a patch ready for (2.) that I'm going to commit today. It might break a few packages on the devel builds, we'll see. Hopefully it won't be too bad. In any case they should be easy to fix.

H.

from summarizedexperiment.

Hi Tuomas, @TuomasBorman

Thank you for providing a detailed report.
It appears that the colData and rowData orderings take precedence over the ordering of the assay.
Perhaps it should be the other way around.
We will investigate and get back to you.

Best regards,
Marcel

from summarizedexperiment.

Done in SummarizedExperiment 1.23.2 (see commit 1e1da5b).

I've manually run R CMD check on a few selected reverse deps: SingleCellExperiment, TreeSummarizedExperiment, zinbwave, VariantAnnotation, xcms, iSEE, velociraptor, and tximeta. All went fine except for a failed test in SingleCellExperiment that I will fix ASAP.

Now SummarizedExperiment has 422 direct reverse deps so I will let the daily builds run all the checks for me. Fingers crossed that we don't see a blood bath on Monday's report (I committed my change too late for Saturday's report and we don't have reports on Sundays).

from summarizedexperiment.

SingleCellExperiment fixed (commit b96ffaff, version 1.15.2)

@LiNk-NY Hey Marcel, this change to the SummarizedExperiment() constructor function causes some of the unit tests in MultiAssayExperiment to fail. Sorry for that. Do you think you can take a look? The easy fix is to remove the offending rownames and/or colnames from the supplied assays (naked assays are always accepted). Thanks!

from summarizedexperiment.

> sessionInfo()
R version 4.1.0 (2021-05-18)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)

Matrix products: default

locale:
[1] LC_COLLATE=English_Finland.1252  LC_CTYPE=English_Finland.1252    LC_MONETARY=English_Finland.1252 LC_NUMERIC=C                    
[5] LC_TIME=English_Finland.1252    

attached base packages:
[1] parallel  stats4    stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] SummarizedExperiment_1.22.0 Biobase_2.52.0              GenomicRanges_1.44.0        GenomeInfoDb_1.28.0         IRanges_2.26.0             
[6] MatrixGenerics_1.4.0        matrixStats_0.59.0          S4Vectors_0.30.0            BiocGenerics_0.38.0        

loaded via a namespace (and not attached):
 [1] lattice_0.20-44        bitops_1.0-7           grid_4.1.0             zlibbioc_1.38.0        XVector_0.32.0         Matrix_1.3-4          
 [7] tools_4.1.0            RCurl_1.98-1.3         DelayedArray_0.18.0    compiler_4.1.0         GenomeInfoDbData_1.2.6

from summarizedexperiment.

@hpages

Sounds good. In my opinion, option 1 would be a nice feature for some users, I guess. However, in addition of being simpler, option 2 fits better to the nature of SE of being just a data container, so no too fancy things should be going on

-Tuomas

from summarizedexperiment.

Other randomly picked up reverse deps that went thru a manual R CMD check on my laptop: AllelicImbalance, batchelor, BiocSklearn, bsseq, CATALYST, ChIPpeakAnno, TCGAutils, and zellkonverter. They all passed successfully.

from summarizedexperiment.