rowData rownames issue explored at BioC2018

[mikelove]

Here is a reproducing example for a potential issue in the devel branch version of SummarizedExperiment. It seems an inconsistency in rownames of rowData and rownames of the SE, which would prevent creation of an SE, is nevertheless allowed during rowData re-assignment.

https://gist.github.com/mikelove/8fb94c610b588d4a6a64848d3fcbdf76

We tried with SummarizedExperiment v1.11.5 (so use.names=TRUE is the default).

Thanks

[hpages]

Thank Mike. I'll look into this.

Note that it has never been a requirement for z to have the same row names as se when doing rowData(se) <- z so the issue is also present in release. What has changed between release and devel is that the rowData() getter propagates the row names by default in devel whereas in release it didn't so you would need to do z <- rowData(se, use.names=TRUE)[5:1,,drop=FALSE] to propagate them to z.

Also note that a SummarizedExperiment object se is a vector-like object along its 1st dimension. This is formalized by the fact that se is a Vector derivative and that length(se) is the same as nrow(se). This also means that, like any other Vector derivative, se can carry metadata columns that can be accessed with mcols(). In the particular case of a SummarizedExperiment object, the metadata columns can also be accessed with rowData(), which is just an alias for mcols(). So it's important to keep mcols() and rowData() consistent and also to keep the behavior of mcols() consistent across all Vector derivatives. In order to do that, we should consider adding the row names check to the mcols() setter. Then the rowData() setter will automatically follow. This is an important change that can possibly affect many packages around. I need to evaluate this.

[mikelove]

I think the broader consequences should certainly be taken into account, and maybe “fixing” this behavior reported above is not worth the downstream effort of many package maintainers.

[hpages]

A compromise could be to only issue a warning that the row names differ and are being ignored. That won't break any code and people who have code that triggers the annoying warning will then decide if they want to remove or fix the row names of z before doing mcols(x) <- z.

I'll evaluate the impact of the 1st solution (i.e. raising an error) and if it turns out to be too disruptive we'll go for the 2nd solution (the warning).

[vondoRishi]

I have encountered almost the same problem and I will try to explain it here based on @mikelove post

# Note I am using newer version SummarizedExperiment 1.20.0

suppressPackageStartupMessages(library(SummarizedExperiment)) m <- matrix(1:20, nrow=5, ncol=4, dimnames=list(letters[1:5], LETTERS[1:4]))

# this gave an error, with old SummarizedExperiment v1.18.2. Probably that was good. # Now it does not give any more error rowdata <- data.frame(x=1:5, row.names=letters[5:1]) se <- SummarizedExperiment(m, rowData=rowdata)

# Reversing the order of rowdata, This also does not gives any errors. rowdata_rev <- rowdata[5:1,] se_rev <- SummarizedExperiment(m, rowData=rowdata_rev) assay(se_rev)["a",]

# Both se and se_rev are constructed with the same information about genes. # However, querying the values of gene "a" here returns different values.

all(assay(se)["a",] == assay(se_rev)["a",])

[vjcitn]

in Bioc 3.14 in devel, your nice example produces:

se <- SummarizedExperiment(m, rowData=rowdata)

*Error in SummarizedExperiment(m, rowData = rowdata) : *

• the rownames and colnames of the supplied assay(s) must be NULL or*

• identical to those of the constructed SummarizedExperiment object*

SummarizedExperiment 1.23.4 has this behavior. I am not sure about backporting of this check to release.

On Wed, Sep 29, 2021 at 11:33 AM Rishi Das Roy @.***> wrote:

I have encountered almost the same problem and I will try to explain it here based on @mikelove https://github.com/mikelove post https://gist.github.com/mikelove/8fb94c610b588d4a6a64848d3fcbdf76

Note I am using newer version SummarizedExperiment 1.20.0

suppressPackageStartupMessages(library(SummarizedExperiment)) m <- matrix(1:20, nrow=5, ncol=4, dimnames=list(letters[1:5], LETTERS[1:4]))

this gave an error, with old SummarizedExperiment v1.18.2. Probably that

was good.

Now it does not give any more error

rowdata <- data.frame(x=1:5, row.names=letters[5:1]) se <- SummarizedExperiment(m, rowData=rowdata)

Reversing the order of rowdata, This also does not gives any errors.

rowdata_rev <- rowdata[5:1,] se_rev <- SummarizedExperiment(m, rowData=rowdata_rev) assay(se_rev)["a",]

Both se and se_rev are constructed with the same information about genes.

However, querying the values of gene "a" here returns different values.

all(assay(se)["a",] == assay(se_rev)["a",])

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/SummarizedExperiment/issues/13#issuecomment-930290596, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDI5QX2PT3BARLJ2N5MNWTUEMWVTANCNFSM4FMD6OVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- The information in this e-mail is intended only for the person to whom it is addressed. If you believe this e-mail was sent to you in error and the e-mail contains patient information, please contact the Partners Compliance  HelpLine at http://www.partners.org/complianceline http://www.partners.org/complianceline . If the e-mail was sent to you in error but does not contain patient information, please contact the sender and properly dispose of the e-mail.

[vondoRishi]

Great!! looking forward to finding these fixes in the next stable release. This is not working with the earlier version 1.22.0. Hopefully, until then the users will find and use the devel.

Thanks for replying.