Skip to content

Commit

Permalink
Merge pull request #990 from single-cell-data/ph/fix/seurat-empty-mat…
Browse files Browse the repository at this point in the history
…rices

[r] Handle empty matrices when creating SOMA from Seurat
  • Loading branch information
mojaveazure authored Feb 23, 2023
2 parents 6f54977 + 4e1babe commit af50af9
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 12 deletions.
2 changes: 1 addition & 1 deletion apis/r/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Description: Interface for working with 'TileDB'-based Stack of Matrices,
from and export to in-memory formats used by popular toolchains like
'Seurat', 'Bioconductor', and even 'AnnData' using the companion Python
package.
Version: 0.1.22.9001
Version: 0.1.22.9002
Authors@R: c(
person(given = "Aaron",
family = "Wolen",
Expand Down
4 changes: 4 additions & 0 deletions apis/r/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# tiledbsoma (development version)

## Changes

- The `SOMA`'s `from_seurat_assay()` method now checks for and skips empty matrices following Seurat's definition of an empty matrix (#990)

## Features

- The `SOMACollection`'s `to_seurat()` method gains a `somas` argument that makes it possible to select a subset of `SOMA`s and `X` layers to be retrieved (#571).
Expand Down
34 changes: 24 additions & 10 deletions apis/r/R/SOMA.R
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,26 @@ SOMA <- R6::R6Class(
= inherits(object, "Assay"),
"'var' must be a logical value" = is.logical(var)
)

if (is.null(layers)) {
layers <- c("counts", "data")
if (seurat_assay_has_scale_data(object)) {
layers <- c(layers, "scale.data")
}
} else {
layers <- match.arg(layers, c("counts", "data", "scale.data"), TRUE)
}

layers <- layers %||% c('counts', 'data', 'scale.data')
layers <- match.arg(
arg = layers,
choices = c('counts', 'data', 'scale.data'),
several.ok = TRUE
)
layers <- Filter(
f = function(x) {
return(!SeuratObject::IsMatrixEmpty(x = SeuratObject::GetAssayData(
object = object,
slot = x
)))
},
x = layers
)
stopifnot(
"None of the layers specified are found in the provided assay" =
as.logical(x = length(x = layers))
)

skip_obs <- self$obs$exists() && is.null(obs)
if (!is.null(obs)) {
stopifnot(
Expand Down Expand Up @@ -452,6 +462,10 @@ SOMA <- R6::R6Class(
# CreateAssayObject only accepts a dgTMatrix matrix for `counts`, 'data'
# and 'scale.data' must be coerced to a dgCMatrix and base::matrix,
# respectively. Bug?
# PH: No, not a bug. The `counts` and `data` matrices must be either a
# dgCMatrix or base::matrix; we call SeuratObject::as.sparse() for
# `counts` and `data` if no one of the above. The `scale.data` slot
# must be a base::matrix and we make no attempts at conversion
assay_obj <- SeuratObject::CreateAssayObject(
data = as(assay_mats$data, "dgCMatrix"),
min.cells = min_cells,
Expand Down
3 changes: 2 additions & 1 deletion apis/r/R/utils-seurat.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ is_seurat_assay <- function(x) {

seurat_assay_has_scale_data <- function(x) {
stopifnot(is_seurat_assay(x))
length(SeuratObject::GetAssayData(x, "scale.data")) > 0
!SeuratObject::IsMatrixEmpty(SeuratObject::GetAssayData(x, 'scale.data'))
# length(SeuratObject::GetAssayData(x, "scale.data")) > 0
}
71 changes: 71 additions & 0 deletions apis/r/tests/testthat/test_SOMA_Seurat.R
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,74 @@ test_that("individual layers can be added or updated", {
expect_identical(soma$X$members$counts$fragment_count(), 1)
expect_identical(soma$X$members$data$fragment_count(), 2)
})

test_that("assays with missing layers are handled: new('matrix')", {
try(tiledb::tiledb_vfs_remove_dir(tdb_uri), silent = TRUE)
assay <- pbmc_small[["RNA"]]
assay <- SeuratObject::SetAssayData(assay, "counts", new("matrix"))
soma <- SOMA$new(uri = tdb_uri)
soma$from_seurat_assay(assay)

expect_equal(names(soma$members$X$members), c("data", "scale.data"))
expect_s4_class(assay1 <- soma$to_seurat_assay(), "Assay")
expect_true(
inherits(mat <- SeuratObject::GetAssayData(assay1, "counts"), "matrix")
)
expect_true(all(dim(mat) == 0L))
expect_true(SeuratObject::IsMatrixEmpty(mat))
})

test_that("assays with missing layers are handled: new('dgCMatrix')", {
try(tiledb::tiledb_vfs_remove_dir(tdb_uri), silent = TRUE)
assay <- pbmc_small[["RNA"]]
assay <- SeuratObject::SetAssayData(assay, "counts", new("dgCMatrix"))
soma <- SOMA$new(uri = tdb_uri)
soma$from_seurat_assay(assay)

expect_equal(names(soma$members$X$members), c("data", "scale.data"))
expect_s4_class(assay1 <- soma$to_seurat_assay(), "Assay")
expect_true(
inherits(mat <- SeuratObject::GetAssayData(assay1, "counts"), "matrix")
)
expect_true(all(dim(mat) == 0L))
expect_true(SeuratObject::IsMatrixEmpty(mat))
})

test_that("assays with missing layers are handled: matrix()", {
try(tiledb::tiledb_vfs_remove_dir(tdb_uri), silent = TRUE)
assay <- pbmc_small[["RNA"]]
assay <- SeuratObject::SetAssayData(assay, "counts", matrix())
soma <- SOMA$new(uri = tdb_uri)
soma$from_seurat_assay(assay)

expect_equal(names(soma$members$X$members), c("data", "scale.data"))
expect_s4_class(assay1 <- soma$to_seurat_assay(), "Assay")
expect_true(
inherits(mat <- SeuratObject::GetAssayData(assay1, "counts"), "matrix")
)
expect_true(all(dim(mat) == 0L))
expect_true(SeuratObject::IsMatrixEmpty(mat))
})

test_that("assays with missing layers are handled: dgCMatrix()", {
try(tiledb::tiledb_vfs_remove_dir(tdb_uri), silent = TRUE)
assay <- pbmc_small[["RNA"]]
expect_s4_class(
m <- as(
as(Matrix::Matrix(NA_real_, sparse = TRUE), "generalMatrix"),
"CsparseMatrix"
),
"dgCMatrix"
)
assay <- SeuratObject::SetAssayData(assay, "counts", new("matrix"))
soma <- SOMA$new(uri = tdb_uri)
soma$from_seurat_assay(assay)

expect_equal(names(soma$members$X$members), c("data", "scale.data"))
expect_s4_class(assay1 <- soma$to_seurat_assay(), "Assay")
expect_true(
inherits(mat <- SeuratObject::GetAssayData(assay1, "counts"), "matrix")
)
expect_true(all(dim(mat) == 0L))
expect_true(SeuratObject::IsMatrixEmpty(mat))
})

0 comments on commit af50af9

Please sign in to comment.