Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for assays stored as SeuratObject::Assay5 #23

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Add support for assays stored as SeuratObject::Assay5 #23

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2424cd
Add support for assays stored as SeuratObject::Assay5
esiegel Dec 3, 2023
d367b88
converting large assays was too slow
esiegel Dec 3, 2023
fbdb3dc
remove strict dependency on seurat 5
esiegel Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Suggests:
testthat (>= 3.0.0),
Matrix
Matrix,
Config/testthat/edition: 3
Imports:
methods,
Seurat,
Seurat (>= 5.0.0),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to support v5 datasets, we need to bump the min version of Seurat. Also, have a direct dependency on SeuratObject because I think using the LayerData method directly reads better.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we keep the Imports on Seurat unversioned and Suggests >= 5.0.0? I am not sure if Roxygen will let you specify it that way, but I imagine cutting to Seurat v5 will trigger a lot of support tickets

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, making v5 a requirement is probably going to be an issue in the near term.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will put in effort to not make this a requirement

SeuratObject (>= 5.0.0),
hdf5r
2 changes: 2 additions & 0 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Generated by roxygen2: do not edit by hand

export(counts_matrix_from_assay)
export(create_bugreport)
export(create_bugreport_from_seurat)
export(create_loupe)
Expand All @@ -16,6 +17,7 @@ importFrom(Seurat,Assays)
importFrom(Seurat,GetAssay)
importFrom(Seurat,Idents)
importFrom(Seurat,Reductions)
importFrom(SeuratObject,LayerData)
importFrom(hdf5r,H5File)
importFrom(hdf5r,H5T_STRING)
importFrom(methods,is)
Expand Down
3 changes: 2 additions & 1 deletion R/err.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ create_bugreport_from_seurat <- function(obj) {
assay <- namedAssay[[1]]
clusters <- select_clusters(obj)
projections <- select_projections(obj)
counts <- counts_matrix_from_assay(assay)

create_bugreport(
assay@counts,
counts,
clusters,
projections,
assay_name = assay_name,
Expand Down
3 changes: 2 additions & 1 deletion R/lib.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ create_loupe_from_seurat <- function(

assay_name <- names(namedAssay)
assay <- namedAssay[[1]]
counts <- counts_matrix_from_assay(assay)

clusters <- select_clusters(obj, dedup=dedup_clusters)
projections <- select_projections(obj)
Expand All @@ -58,7 +59,7 @@ create_loupe_from_seurat <- function(
}

success <- create_loupe(
assay@counts,
counts,
clusters=clusters,
projections=projections,
output_dir=output_dir,
Expand Down
20 changes: 19 additions & 1 deletion R/util.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ select_assay <- function(obj) {
for (i in seq_along(assay_priority)) {
name <- names(assay_priority[i])
assay <- Seurat::GetAssay(obj, assay=name)
counts <- counts_matrix_from_assay(assay)

if (length(assay@counts) > 0) {
if (length(counts) > 0) {
result = list()
result[[name]] = assay
return(result)
Expand All @@ -67,6 +68,23 @@ select_assay <- function(obj) {
NULL
}

#' Extract the counts matrix from the Assay
#'
#' @param assay A SeuratObject::Assay or SeuratObject::Assay5
#'
#' @return A sparse counts matrix
#'
#' @importFrom SeuratObject LayerData
#'
#' @export
counts_matrix_from_assay <- function(assay) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this method to get the counts matrix from either of the assay versions instead of updating the select_assay method to just return the counts matrix directly. Doing this so that I don't break anyone's code if they are relying on select_assay

# Support both Assay and Assay5 classes
# The same as using the helper `assay$counts` but less indirection.
# With Assay5 it is important to use this method as oppossed to grabbing
# the data from `assay@layers` directly as that matrix won't contain the dimnames
SeuratObject::LayerData(assay, "counts")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the above suggestion about keeping Seurat dependency unversioned, you could probably...

if (unlist(packageVersion("Seurat"))[1] > 5) {
  SeuratObject::LayerData(assay, "counts")
} else {
  assay@counts
}

Although I am not sure how that would interact with NAMESPACE

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this:

if (packageVersion("Seurat") >= 5) {
  SeuratObject::LayerData(assay, "counts")
} else {
  if (is(assay, 'Assay5')) {
    stop("Cannot get count matrix: Please upgrade to Seurat > 5 to support dataset")
  }

  assay@counts
}

It looks like it is possible to have a Seurat object with a mix of assay versions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way they get into the else if is if they download a Seurat 5 dataset while on Seurat < 5, right? I think that is a fine condition to stop on. Make sure to unlist the return of packageVersion, though (it is a list with each entry being a corresponding x/y/z)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is my understanding

}

#' Select clusters from the assay
#'
#' @param obj A Seurat Object
Expand Down
5 changes: 4 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ library("loupeR")
# Gene Expression RNA assay
assay <- seurat_obj[["RNA"]]

# get counts matrix from either the old or newer formats of assay
counts <- counts_matrix_from_assay(assay)

# convert the count matrix, clusters, and projections into a Loupe file
create_loupe(
assay@counts,
counts,
clusters = select_clusters(seurat_obj),
projections = select_projections(seurat_obj)
)
Expand Down
17 changes: 17 additions & 0 deletions man/counts_matrix_from_assay.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 19 additions & 5 deletions tests/testthat/test-util.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,29 @@ test_that("select_assay selects active assay", {
rna3 <- create_count_mat(5, 5)

obj <- Seurat::CreateSeuratObject(rna1, assay="rna1")
obj[["rna2_"]] = Seurat::CreateAssayObject(rna2, key="rna2_")
obj[["rna3_"]] = Seurat::CreateAssayObject(rna3, key="rna3_")
obj[["rna2"]] = Seurat::CreateAssayObject(rna2, key="rna2_")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, but the names here didn't need to have an underscore even though the key parameter to CreateAssayObject requires it

obj[["rna3"]] = Seurat::CreateAssayObject(rna3, key="rna3_")

expect_equal(Seurat::DefaultAssay(object = obj), "rna1")
Seurat::DefaultAssay(object = obj) <- "rna2_"
expect_equal(Seurat::DefaultAssay(object = obj), "rna2_")
Seurat::DefaultAssay(object = obj) <- "rna2"
expect_equal(Seurat::DefaultAssay(object = obj), "rna2")

assay <- select_assay(obj)
expect_equal(names(assay), "rna2_")
expect_equal(names(assay), "rna2")
})

test_that("counts_matrix_from_assay works on different assay version", {
rna1 <- create_count_mat(5, 5)
rna2 <- create_count_mat(5, 5)
rna3 <- create_count_mat(5, 5)

obj <- Seurat::CreateSeuratObject(rna1, assay="rna1")
obj[["rna2"]] = Seurat::CreateAssayObject(rna2, key="rna2_")
obj[["rna3"]] = SeuratObject::CreateAssay5Object(rna3, key="rna3_")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seurat package doesn't export the underlying v5 create method like it does with the non v5.

Comment on lines +19 to +25
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rna1 <- create_count_mat(5, 5)
rna2 <- create_count_mat(5, 5)
rna3 <- create_count_mat(5, 5)
obj <- Seurat::CreateSeuratObject(rna1, assay="rna1")
obj[["rna2"]] = Seurat::CreateAssayObject(rna2, key="rna2_")
obj[["rna3"]] = SeuratObject::CreateAssay5Object(rna3, key="rna3_")
rna <- create_count_mat(5, 5)
obj <- Seurat::CreateSeuratObject(rna, assay="rna1")
obj[["rna2"]] = Seurat::CreateAssayObject(rna, key="rna2_")
obj[["rna3"]] = SeuratObject::CreateAssay5Object(rna, key="rna3_")

R will pass function arguments by value rather than reference so you shouldn't need to construct multiple instances yourself

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually want to create three different matrices because I want their values to be different for the tests. Turns out R is copy by value, but only if the value is modified.


expect_equal(counts_matrix_from_assay(obj[["rna1"]]), rna1)
expect_equal(counts_matrix_from_assay(obj[["rna2"]]), rna2)
expect_equal(counts_matrix_from_assay(obj[["rna3"]]), rna3)
})

test_that("select_clusters selects Idents", {
Expand Down