Merge remote-tracking branch 'origin/main' into add-tests

rcannood · rcannood · commit 131a828591c7 · 2025-08-05T15:13:09.000+02:00
diff --git a/NEWS.md b/NEWS.md
@@ -2,7 +2,8 @@
 
 - Bump required R version to 4.5 in preparation for Bioconductor submission
   (PR #309)
-- Remove deprecated functions and arguments (PR #311)
+- Remove deprecated functions and arguments (PR #311, PR #313)
+- Minor cleanups and improvements (PR #313).
 - Add more tests to increase coverage (PR #315)
 
 # anndataR 0.2.0
diff --git a/R/AbstractAnnData.R b/R/AbstractAnnData.R
@@ -272,23 +272,6 @@ AbstractAnnData <- R6::R6Class(
       )
     },
     #' @description
-    #' `r lifecycle::badge('deprecated')`
-    #'
-    #' Deprecated, use `as_HDF5AnnData()` instead
-    #'
-    #' @param ... Arguments passed to `adata$as_HDF5AnnData()`
-    #'
-    #' @return An [`HDF5AnnData`] object
-    to_HDF5AnnData = function(...) {
-      lifecycle::deprecate_warn(
-        "0.1.0",
-        "to_HDF5AnnDAta()",
-        "as_HDF5AnnData()",
-      )
-
-      self$as_HDF5AnnData(...)
-    },
-    #' @description
     #' Write the `AnnData` object to an H5AD file
     #'
     #' See [write_h5ad()] for details
diff --git a/R/as_Seurat.R b/R/as_Seurat.R
@@ -352,9 +352,9 @@ as_Seurat <- function(
     !"counts" %in% names(layers_mapping) &&
       !"data" %in% names(layers_mapping)
   ) {
-    cli_warn(c(
+    cli_warn(paste(
       "No {.val counts} or {.val data} layer found in {.arg names(layers_mapping)},",
-      "this may lead to unexpected results when using the resulting Seurat object."
+      "this may lead to unexpected results when using the resulting {.cls Seurat} object."
     ))
   }
 
@@ -381,6 +381,7 @@ as_Seurat <- function(
   # and remove the counts layer
   if (!"counts" %in% names(layers_mapping)) {
     SeuratObject::LayerData(obj, layer = dummy_counts) <- counts
+    SeuratObject::DefaultLayer(obj[[assay_name]]) <- dummy_counts
     obj[[assay_name]]$counts <- NULL
   }
 
diff --git a/R/from_Seurat.R b/R/from_Seurat.R
@@ -33,12 +33,6 @@ from_Seurat <- function(
   output_class = c("InMemory", "HDF5AnnData"),
   ...
 ) {
-  lifecycle::deprecate_soft(
-    when = "0.99.0",
-    what = "from_Seurat()",
-    with = "as_AnnData()"
-  )
-
   check_requires("Converting Seurat to AnnData", c("SeuratObject", "Seurat"))
 
   output_class <- match.arg(output_class)
diff --git a/R/from_SingleCellExperiment.R b/R/from_SingleCellExperiment.R
@@ -31,12 +31,6 @@ from_SingleCellExperiment <- function(
   output_class = c("InMemory", "HDF5AnnData"),
   ...
 ) {
-  lifecycle::deprecate_soft(
-    when = "0.99.0",
-    what = "from_SingleCellExperiment()",
-    with = "as_AnnData()"
-  )
-
   check_requires(
     "Converting SingleCellExperiment to AnnData",
     "SingleCellExperiment",
diff --git a/R/generate_dataset.R b/R/generate_dataset.R
@@ -24,11 +24,6 @@
 #'
 #' @examples
 #' dummy <- generate_dataset()
-#' \dontrun{
-#' dummy <- generate_dataset(format = "AnnData")
-#' dummy <- generate_dataset(format = "SingleCellExperiment")
-#' dummy <- generate_dataset(format = "Seurat")
-#' }
 generate_dataset <- function(
   n_obs = 10L,
   n_vars = 20L,
diff --git a/man/AbstractAnnData.Rd b/man/AbstractAnnData.Rd
diff --git a/man/HDF5AnnData.Rd b/man/HDF5AnnData.Rd
diff --git a/man/InMemoryAnnData.Rd b/man/InMemoryAnnData.Rd
diff --git a/man/generate_dataset.Rd b/man/generate_dataset.Rd
diff --git a/tests/testthat/helper-expect_equal_py.R b/tests/testthat/helper-expect_equal_py.R
@@ -4,7 +4,8 @@ expect_equal_py <- function(a, b) {
 
   bi <- reticulate::import_builtins()
 
-  testthat::expect_equal(bi$str(bi$type(a)), bi$str(bi$type(b))) # does this always work?
+  # ↓ does this always work?
+  testthat::expect_equal(bi$str(bi$type(a)), bi$str(bi$type(b)))
 
   if (inherits(a, "pandas.core.frame.DataFrame")) {
     pd <- reticulate::import("pandas")
diff --git a/tests/testthat/test-as_Seurat.R b/tests/testthat/test-as_Seurat.R
@@ -11,6 +11,12 @@ known_issues <- read_known_issues()
 ad <- generate_dataset(n_obs = 10L, n_vars = 20L, format = "AnnData")
 ad$obsm[["X_pca"]] <- matrix(1:50, 10, 5)
 ad$varm[["PCs"]] <- matrix(1:100, 20, 5)
+ad$layers["counts"] <- ad$layers["numeric_csparse"]
+
+# Remove column names to avoid warnings
+for (name in c("integer", "numeric", "integer_with_nas", "numeric_with_nas")) {
+  colnames(ad$obsm[[name]]) <- NULL
+}
 
 seu <- ad$as_Seurat()
 
@@ -137,7 +143,6 @@ test_that("as_Seurat retains pca dimred", {
 test_that("as_Seurat works with list mappings", {
   expect_no_error(
     ad$as_Seurat(
-      x_mapping = "counts",
       object_metadata_mapping = as.list(.as_Seurat_guess_object_metadata(ad)),
       layers_mapping = as.list(.as_Seurat_guess_layers(ad)),
       assay_metadata_mapping = as.list(.as_Seurat_guess_assay_metadata(ad)),
@@ -156,10 +161,7 @@ test_that("as_Seurat works with list mappings", {
 
 test_that("as_Seurat works with a vector reduction_mapping", {
   expect_no_error(
-    ad$as_Seurat(
-      x_mapping = "counts",
-      reduction_mapping = c(numeric = "numeric_matrix")
-    )
+    ad$as_Seurat(reduction_mapping = c(numeric = "numeric_matrix"))
   )
 })
 
@@ -169,7 +171,7 @@ test_that("as_Seurat works with unnamed mappings", {
       object_metadata_mapping = unname(.as_Seurat_guess_object_metadata(ad)),
       layers_mapping = c(
         na.omit(unname(.as_Seurat_guess_layers(ad))),
-        counts = NA
+        data = NA
       ),
       assay_metadata_mapping = unname(.as_Seurat_guess_assay_metadata(ad)),
       graph_mapping = unname(.as_Seurat_guess_graphs(ad)),
diff --git a/tests/testthat/test-from_Seurat.R b/tests/testthat/test-from_Seurat.R
@@ -1,18 +1,18 @@
 skip_if_not_installed("Seurat")
 library(Seurat)
 
+`%||%` <- rlang::`%||%`
+
 known_issues <- read_known_issues()
 
-suppressWarnings({
-  counts <- matrix(rbinom(20000, 1000, .001), nrow = 100)
-  obj <- CreateSeuratObject(counts = counts)
-  obj <- NormalizeData(obj)
-  obj <- FindVariableFeatures(obj)
-  obj <- ScaleData(obj)
-  obj <- RunPCA(obj, npcs = 10L)
-  obj <- FindNeighbors(obj)
-  obj <- RunUMAP(obj, dims = 1:10)
-})
+counts <- matrix(rbinom(20000, 1000, .001), nrow = 100)
+obj <- CreateSeuratObject(counts = as(counts, "CsparseMatrix"))
+obj <- NormalizeData(obj, verbose = FALSE)
+obj <- FindVariableFeatures(obj, verbose = FALSE)
+obj <- ScaleData(obj, verbose = FALSE)
+obj <- RunPCA(obj, npcs = 10L, verbose = FALSE)
+obj <- FindNeighbors(obj, verbose = FALSE)
+obj <- RunUMAP(obj, dims = 1:10, verbose = FALSE)
 
 active_assay <- obj[[DefaultAssay(obj)]]
 
@@ -199,8 +199,16 @@ test_that("as_AnnData (Seurat) works with list mappings", {
       obsm_mapping = as.list(.from_Seurat_guess_obsms(obj, active_assay)),
       varm_mapping = as.list(.from_Seurat_guess_varms(obj, active_assay)),
       obsp_mapping = as.list(.from_Seurat_guess_obsps(obj, active_assay)),
-      varp_mapping = as.list(.from_Seurat_guess_varps(obj)),
-      uns_mapping = as.list(.from_Seurat_guess_uns(obj))
+      varp_mapping = if (length(as.list(.from_Seurat_guess_varps(obj))) > 0) {
+        as.list(.from_Seurat_guess_varps(obj))
+      } else {
+        FALSE
+      },
+      uns_mapping = if (length(as.list(.from_Seurat_guess_uns(obj))) > 0) {
+        as.list(.from_Seurat_guess_uns(obj))
+      } else {
+        FALSE
+      }
     )
   )
 })
@@ -216,8 +224,8 @@ test_that("as_AnnData (Seurat) works with unnamed mappings", {
       obsm_mapping = unname(.from_Seurat_guess_obsms(obj, active_assay)),
       varm_mapping = unname(.from_Seurat_guess_varms(obj, active_assay)),
       obsp_mapping = unname(.from_Seurat_guess_obsps(obj, active_assay)),
-      varp_mapping = unname(.from_Seurat_guess_varps(obj)),
-      uns_mapping = unname(.from_Seurat_guess_uns(obj))
+      varp_mapping = unname(.from_Seurat_guess_varps(obj)) %||% FALSE,
+      uns_mapping = unname(.from_Seurat_guess_uns(obj)) %||% FALSE
     )
   )
 })
diff --git a/tests/testthat/test-roundtrip-X.R b/tests/testthat/test-roundtrip-X.R
@@ -64,7 +64,6 @@ for (name in test_names) {
     expect_equal(str_r, str_py)
   })
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0("Comparing an anndata with X '", name, "' with reticulate works"),
     {
diff --git a/tests/testthat/test-roundtrip-layers.R b/tests/testthat/test-roundtrip-layers.R
@@ -70,7 +70,6 @@ for (name in test_names) {
 
   gc()
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0(
       "Comparing an anndata with layer '",
diff --git a/tests/testthat/test-roundtrip-obsmvarm.R b/tests/testthat/test-roundtrip-obsmvarm.R
@@ -92,7 +92,6 @@ for (name in test_names) {
     }
   )
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0(
       "Comparing an anndata with obsm and varm '",
diff --git a/tests/testthat/test-roundtrip-obspvarp.R b/tests/testthat/test-roundtrip-obspvarp.R
@@ -75,7 +75,6 @@ for (name in test_names) {
     }
   )
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0(
       "Comparing an anndata with obsp and varp '",
diff --git a/tests/testthat/test-roundtrip-obsvar.R b/tests/testthat/test-roundtrip-obsvar.R
@@ -72,7 +72,6 @@ for (name in test_names) {
     expect_equal(str_r, str_py)
   })
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0(
       "Comparing an anndata with obs and var '",
diff --git a/tests/testthat/test-roundtrip-uns-nested.R b/tests/testthat/test-roundtrip-uns-nested.R
@@ -66,7 +66,6 @@ for (name in test_names) {
     expect_equal(str_r, str_py)
   })
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0(
       "Comparing an anndata with uns_nested '",
diff --git a/tests/testthat/test-roundtrip-uns.R b/tests/testthat/test-roundtrip-uns.R
@@ -66,7 +66,6 @@ for (name in test_names) {
     expect_equal(str_r, str_py)
   })
 
-  # maybe this test simply shouldn't be run if there is a known issue with reticulate
   test_that(
     paste0("Comparing an anndata with uns '", name, "' with reticulate works"),
     {
diff --git a/vignettes/known_issues.Rmd b/vignettes/known_issues.Rmd
@@ -10,8 +10,8 @@ vignette: >
 This document lists known issues with the package and suggests possible solutions.
 
 ```{r known-issues, echo=FALSE, results="asis"}
-# how to get a file from a package
-data <- yaml::read_yaml(system.file("known_issues.yaml", package = "anndataR"))$known_issues
+known_issues_path <- system.file("known_issues.yaml", package = "anndataR")
+data <- yaml::read_yaml(known_issues_path)$known_issues
 
 for (i in seq_along(data)) {
   str <- paste0(
