Bioconductor / RaggedExperiment

Matrix-like representations of mutation and CN data
https://bioconductor.org/packages/RaggedExperiment
4 stars 3 forks source link

dimnames(x) <- value was not working as expected for RaggedExperiment #29

Closed yli110-stat697 closed 1 year ago

yli110-stat697 commented 1 year ago

Hi, I have a RaggedExperiment and I want to change its colnames(x) or row names of its colData(x) to a different sample id from its colData, and found that the dimnames() setter was not working as it claimed from the documentation.

Example RaggedExperiment

library(RaggedExperiment)
sample1 <- GRanges(c(featureA = "chr1:1-10:-", 
                     featureB = "chr1:8-14:+", 
                     featureC = "chr2:15-18:+"),
                   score = 3:5,
                   cpnum = 6:8)
sample2 <- GRanges(c(featureD = "chr1:1-10:-", featureE = "chr2:11-18:+"),
                   score = 1:2,
                   cpnum = 9:10)

colDat <- DataFrame(Sample_id = paste0("sample", 1:2),
                    New_id = paste0("sample_and_patient", 1:2),
                    Tissue = c("Lung", "Liver"),
                    Condition = c("Turmo", "Normal"))

# Assemble RaggedExperiment object
ragexp <- RaggedExperiment(sample1 = sample1,
                           sample2 = sample2,
                           colData = colDat)

# Add metadata
metadata(ragexp) <- list(genome_build = "hg19")

ragexp

Currently the colnames are sample1, sample2, but I want to change to the New_id from colDat. I was trying the dimnames and fount that

# getter works as expected
ragexp_dimnames <- dimnames(ragexp)

# try update the dimnames
ragexp_dimnames[[2]] <- colDat$New_id

# reset dimnames not working
dimnames(ragexp) <- ragexp_dimnames
Screenshot 2023-08-07 at 9 54 47 AM
LiNk-NY commented 1 year ago

Hi @yli110-stat697 Rosemary,

Thank you for reporting. This seems to be an issue with DataFrame sub-assignment. I have asked Hervé about it here https://github.com/Bioconductor/S4Vectors/issues/117

-Marcel

hpages commented 1 year ago

dimnames<- seems broken when the RaggedExperiment object carries colData:

library(RaggedExperiment)

sample1 <- GRanges(c("chr1:1-10", "chr1:11-18"), score=1:2)
sample2 <- GRanges(c("chr1:1-10", "chr2:11-18"), score=3:4)
re <- RaggedExperiment(sample1, sample2)
colData(re) <- DataFrame(sampleID=c("ID1", "ID2"))
dimnames(re) <- dimnames(re)
# Error in .subassign_columns(x, nsbs, value) : 
#   provided 1 variables to replace 0 variables

Here's another issue with the current implementation of dimnames<-:

re2 <- RaggedExperiment(sample1, sample2)[4:1]

rownames(re2) <- letters[1:4]
rownames(re2)
# [1] "d" "c" "b" "a"    <-- not the expected rownames

FWIW it looks like the current implementation of the dimnames<- method is unnecessarily complicated and could be made slightly simpler by doing something like this:

.set_RaggedExperiment_dimnames <- function(x, value)
{
    new_rownames <- value[[1L]]
    new_colnames <- value[[2L]]
    x_assays <- .assays(x)
    if (is.null(new_colnames)) {
        names(x_assays) <- NULL
    } else {
        names(x_assays)[.colidx(x)] <- new_colnames
    }
    unlisted_assays <- unlist(x_assays, use.names=FALSE)
    if (is.null(new_rownames)) {
        names(unlisted_assays) <- NULL
    } else {
        names(unlisted_assays)[.rowidx(x)] <- new_rownames
    }
    new_assays <- relist(unlisted_assays, x_assays)
    mcols(new_assays) <- mcols(x_assays)
    BiocGenerics:::replaceSlots(x, assays=new_assays, check=FALSE)
}

Hope this helps,

H.

hpages commented 1 year ago

I just edited my .set_RaggedExperiment_dimnames proposal above to make sure that setting the dimnames to NULL actually drops the dimnames. Note that the current dimnames<- method also fails to handle this case:

re <- RaggedExperiment(sample1, sample2)
dimnames(re) <- list(letters[1:4], LETTERS[1:2])
dimnames(re) <- NULL
dimnames(re)
# [[1]]
# [1] "a" "b" "c" "d"
# 
# [[2]]
# [1] "A" "B"

The unit tests for dimnames<- would need to be expanded to cover all these situations.

LiNk-NY commented 1 year ago

Thank you. This should be fixed in the latest version of RaggedExperiment (1.24.2 or 1.25.3).