diff --git a/R/merge_sce_list.R b/R/merge_sce_list.R index 6a2b8ec9..5a0f3954 100644 --- a/R/merge_sce_list.R +++ b/R/merge_sce_list.R @@ -61,8 +61,6 @@ merge_sce_list <- function( preserve_rowdata_cols = NULL, cell_id_column = "cell_id", include_altexp = TRUE) { - - ## Checks -------------------------- if (is.null(names(sce_list))) { warning( glue::glue( @@ -70,7 +68,7 @@ merge_sce_list <- function( named based on their list index in the merged SCE object." ) ) - names(sce_list) <- 1:length(sce_list) + names(sce_list) <- seq_along(sce_list) } if (length(sce_list) < 2) { @@ -96,13 +94,14 @@ merge_sce_list <- function( coldata_suffixes <- c("sum", "detected", "percent") altexp_columns <- glue::glue("altexps_{names(altexp_attributes)}") |> purrr::map( - \(prefix) { glue::glue("{prefix}_{coldata_suffixes}")} + \(prefix) { + glue::glue("{prefix}_{coldata_suffixes}") + } ) |> unlist() # Update retain_coldata_cols retain_coldata_cols <- c(retain_coldata_cols, altexp_columns) - } else { # Remove altexps if we are not including them sce_list <- sce_list |> @@ -139,7 +138,7 @@ merge_sce_list <- function( # check that library id and sample id are present in metadata id_checks <- sce_list |> - purrr::map(\(sce){ + purrr::map(\(sce) { all(c("library_id", "sample_id") %in% names(metadata(sce))) }) |> unlist() @@ -196,9 +195,7 @@ merge_sce_list <- function( # If we are including altExps, process them and save to list to add to merged SCE if (include_altexp) { - for (altexp_name in names(altexp_attributes)) { - expected_assays <- altexp_attributes[[altexp_name]][["assays"]] expected_features <- altexp_attributes[[altexp_name]][["features"]] @@ -215,7 +212,7 @@ merge_sce_list <- function( } # Create the merged SCE from the processed list - merged_sce <- do.call(cbind, sce_list) + merged_sce <- do.call(combineCols, unname(sce_list)) # Replace existing metadata list with merged metadata metadata(merged_sce) <- metadata_list @@ -245,15 +242,14 @@ merge_sce_list <- function( #' #' @return An updated SCE that is prepared for merging prepare_sce_for_merge <- function( - sce, - sce_name, - batch_column, - cell_id_column, - shared_features, - retain_coldata_cols, - preserve_rowdata_cols, - is_altexp = FALSE) { - + sce, + sce_name, + batch_column, + cell_id_column, + shared_features, + retain_coldata_cols, + preserve_rowdata_cols, + is_altexp = FALSE) { # Subset to shared features sce <- sce[shared_features, ] @@ -337,18 +333,8 @@ prepare_altexp_for_merge <- function( batch_column, cell_id_column, preserve_rowdata_cols = c("target_type")) { - - if (!altexp_name %in% altExpNames(sce) ) { - - na_assays <- expected_assays |> - purrr::set_names() |> - purrr::map( - build_na_matrix, - expected_features, - colnames(sce) - ) - - altExp(sce, altexp_name) <- SingleCellExperiment(assays = na_assays) + if (!altexp_name %in% altExpNames(sce)) { + return(sce) } # Now, prepare this altexp for merge @@ -374,10 +360,12 @@ prepare_altexp_for_merge <- function( #' #' @return Updated metadata list to store in the SCE update_sce_metadata <- function(metadata_list) { - # first check that this library hasn't already been merged if ("library_metadata" %in% names(metadata_list)) { - stop("This SCE object appears to be a merged object. We do not support merging objects with objects that have already been merged.") + stop(paste( + "This SCE object appears to be a merged object", + "We do not support merging objects with objects that have already been merged." + )) } # create library and sample metadata. @@ -398,33 +386,6 @@ update_sce_metadata <- function(metadata_list) { } - - -#' Create a sparse matrix with all NA values -#' -#' @param assay_name Intended assay name for this matrix (e.g., "counts") -#' @param matrix_rownames Vector of matrix rown ames -#' @param matrix_colnames Vector of matrix column names -#' -#' @return Sparse matrix -build_na_matrix <- function( - assay_name, - matrix_rownames, - matrix_colnames) { - - Matrix::Matrix( - data = NA_real_, - nrow = length(matrix_rownames), - ncol = length(matrix_colnames), - dimnames = list( - matrix_rownames, - matrix_colnames - ), - sparse = TRUE - ) -} - - #' Helper function to check altExp compatibility #' #' @param sce_list List of SCEs with altExps to check @@ -433,7 +394,6 @@ build_na_matrix <- function( #' with each sublist formatted as: #' altexp_name = list(features = c(features), assays = c(assays)) get_altexp_attributes <- function(sce_list) { - # Attribute list to save for later use altexp_attributes <- list() @@ -445,7 +405,6 @@ get_altexp_attributes <- function(sce_list) { # For each in altexp_names (if present), do they have the same features? # If not, error out for (altexp_name in altexp_names) { - # all altExps for this name altexp_list <- sce_list |> purrr::keep(\(sce) altexp_name %in% altExpNames(sce)) |> @@ -473,25 +432,11 @@ get_altexp_attributes <- function(sce_list) { purrr::map(assayNames) |> purrr::reduce(union) - # create logical vector for presence of all assays - assays_present <- altexp_list |> - purrr::map_lgl( - \(alt_sce) setequal(assayNames(alt_sce), all_assays) - ) - - # TODO: we may want to drop assays that aren't present in all altexps, rather than dying. - if (!all(assays_present)) { - stop( - glue::glue("The {altexp_name} alternative experiments do not share the same set of assays.") - ) - } - # Save to altexp_attributes for later use altexp_attributes[[altexp_name]] <- list( "features" = all_features, "assays" = all_assays ) - } return(altexp_attributes) } diff --git a/man/build_na_matrix.Rd b/man/build_na_matrix.Rd deleted file mode 100644 index 45fc6b9b..00000000 --- a/man/build_na_matrix.Rd +++ /dev/null @@ -1,21 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/merge_sce_list.R -\name{build_na_matrix} -\alias{build_na_matrix} -\title{Create a sparse matrix with all NA values} -\usage{ -build_na_matrix(assay_name, matrix_rownames, matrix_colnames) -} -\arguments{ -\item{assay_name}{Intended assay name for this matrix (e.g., "counts")} - -\item{matrix_rownames}{Vector of matrix rown ames} - -\item{matrix_colnames}{Vector of matrix column names} -} -\value{ -Sparse matrix -} -\description{ -Create a sparse matrix with all NA values -} diff --git a/tests/testthat/test-merge_sce_list.R b/tests/testthat/test-merge_sce_list.R index 069c2eb6..e546eb89 100644 --- a/tests/testthat/test-merge_sce_list.R +++ b/tests/testthat/test-merge_sce_list.R @@ -60,7 +60,6 @@ sce_list <- list( # Tests without altexps ---------------------------------------------- test_that("`update_sce_metadata()` returns the expected list", { - metadata_list <- metadata(sce_list[[1]]) new_metadata <- update_sce_metadata(metadata_list) @@ -72,7 +71,7 @@ test_that("`update_sce_metadata()` returns the expected list", { expect_equal( names(new_metadata$library_metadata), - c("library_id", "sample_id", "total_reads") + c("library_id", "sample_id", "total_reads") ) }) @@ -327,7 +326,7 @@ test_that("merging SCEs with library metadata fails as expected, no altexps", { expect_error( merge_sce_list( sce_list - ) + ) ) }) @@ -341,15 +340,22 @@ add_sce_altexp <- function( altexp_name, num_altexp_features, n_cells) { - sce_alt <- sim_sce( n_genes = num_altexp_features, n_cells = n_cells, - n_empty = 0) + n_empty = 0 + ) # ensure matching barcodes colnames(sce_alt) <- colnames(sce) + # change feature names + rownames(sce_alt) <- stringr::str_replace( + rownames(sce_alt), + "^GENE", + toupper(altexp_name) + ) + # add some rowdata columns rowData(sce_alt)[["target_type"]] <- "target" # should be retained rowData(sce_alt)[["feature_column"]] <- rownames(sce_alt) @@ -395,7 +401,6 @@ full_altexp_features <- rownames(altExp(sce_list_with_altexp[[1]])) test_that("prepare_sce_for_merge() works as expected with is_altexp=TRUE", { - test_altexp <- altExp(sce_list_with_altexp[[1]]) prepared_altexp <- prepare_sce_for_merge( test_altexp, @@ -424,51 +429,7 @@ test_that("prepare_sce_for_merge() works as expected with is_altexp=TRUE", { }) -test_that("prepare_sce_for_merge() works from an NA matrix", { - - - assay_names <- c("counts", "logcounts") - sce <- sce_list_with_altexp[[1]] - na_assays <- assay_names |> - purrr::set_names() |> - purrr::map( - build_na_matrix, - full_altexp_features, - colnames(sce) - ) - test_altexp <- SingleCellExperiment(assays = na_assays) - - prepared_altexp <- prepare_sce_for_merge( - test_altexp, - "test", - batch_column = batch_column, - cell_id_column = cell_id_column, - shared_features = full_altexp_features, - retain_coldata_cols = NULL, - preserve_rowdata_cols = c("target_type"), - is_altexp = TRUE - ) - - expect_equal( - assayNames(prepared_altexp), assay_names - ) - - expect_equal( - colnames(colData(prepared_altexp)), c("batch", "cell_id") - ) - - expect_equal( - colnames(prepared_altexp), colnames(sce) - ) - - expect_equal( - rownames(prepared_altexp), full_altexp_features - ) -}) - - test_that("merging SCEs with altExps works as expected when include_altexps = FALSE", { - merged_sce <- merge_sce_list( sce_list_with_altexp, batch_column = batch_column, @@ -481,11 +442,9 @@ test_that("merging SCEs with altExps works as expected when include_altexps = FA # there should not be any altExps expect_length(altExpNames(merged_sce), 0) - }) test_that("merging SCEs with 1 altexp and same features works as expected, with altexps", { - merged_sce <- merge_sce_list( sce_list_with_altexp, batch_column = batch_column, @@ -511,13 +470,15 @@ test_that("merging SCEs with 1 altexp and same features works as expected, with expect_equal(colnames(merged_altexp), expected_colnames) # Check colData columns - expected_coldata <- c("sum", - "detected", - "altexps_adt_sum", - "altexps_adt_detected", - "altexps_adt_percent", - batch_column, - "cell_id") + expected_coldata <- c( + "sum", + "detected", + "altexps_adt_sum", + "altexps_adt_detected", + "altexps_adt_percent", + batch_column, + "cell_id" + ) expect_true( setequal( @@ -525,16 +486,14 @@ test_that("merging SCEs with 1 altexp and same features works as expected, with expected_coldata ) ) - }) test_that("merging SCEs with 1 altexp but different features fails as expected, with altexps", { - # keep only the first 3 features from the first SCE - altExp(sce_list_with_altexp[[1]]) <- altExp(sce_list_with_altexp[[1]])[1:3,] + altExp(sce_list_with_altexp[[1]]) <- altExp(sce_list_with_altexp[[1]])[1:3, ] expect_error( @@ -553,12 +512,11 @@ test_that("merging SCEs with 1 altexp but different features fails as expected, test_that("merging SCEs where 1 altExp is missing works as expected, with altexps", { - sce_list_with_altexp[["sce4"]] <- removeAltExps(sce_list_with_altexp[[1]]) # from cbind docs: - #The colnames in colData(SummarizedExperiment) must match or an error is thrown. - #Duplicate columns of rowData(SummarizedExperiment) must contain the same data. + # The colnames in colData(SummarizedExperiment) must match or an error is thrown. + # Duplicate columns of rowData(SummarizedExperiment) must contain the same data. merged_sce <- merge_sce_list( sce_list_with_altexp, @@ -570,37 +528,12 @@ test_that("merging SCEs where 1 altExp is missing works as expected, with altexp ) expect_equal(altExpNames(merged_sce), "adt") - }) ## Other tests ------------------ -test_that("build_na_matrix works as expected",{ - - rows <- letters[1:5] - cols <- letters[6:10] - sparse_mat <- build_na_matrix( - "name", - rows, - cols - ) - - expect_s4_class(sparse_mat, "sparseMatrix") - - expect_equal( - rownames(sparse_mat), rows - ) - - expect_equal( - colnames(sparse_mat), cols - ) - -}) - - test_that("get_altexp_attributes passes when it should pass", { - attribute_list <- get_altexp_attributes(sce_list_with_altexp) expect_equal( attribute_list[["adt"]][["assays"]], c("counts", "logcounts") @@ -608,20 +541,10 @@ test_that("get_altexp_attributes passes when it should pass", { expect_equal( attribute_list[["adt"]][["features"]], full_altexp_features ) - }) -test_that("get_altexp_attributes throws an error as expected when assays do not match", { - - logcounts(altExp(sce_list_with_altexp[[3]])) <- NULL - expect_error(get_altexp_attributes(sce_list_with_altexp)) - -}) - test_that("get_altexp_attributes throws an error as expected when features do not match", { - - altExp(sce_list_with_altexp[[1]]) <- altExp(sce_list_with_altexp[[1]])[1:3,] + altExp(sce_list_with_altexp[[1]]) <- altExp(sce_list_with_altexp[[1]])[1:3, ] expect_error(get_altexp_attributes(sce_list_with_altexp)) - })