From a63894d34bfecfa8b08c01d7d5e096ed8a622bb2 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Tue, 19 Sep 2023 15:24:13 +0200 Subject: [PATCH 01/11] add uns --- R/AbstractAnnData.R | 19 +++++++++++++++++++ R/AnnData.R | 8 ++++++-- R/HDF5AnnData.R | 29 ++++++++++++++++++++++------- R/InMemoryAnnData.R | 12 +++++++++--- 4 files changed, 56 insertions(+), 12 deletions(-) diff --git a/R/AbstractAnnData.R b/R/AbstractAnnData.R index 754b5786..e2ddd44c 100644 --- a/R/AbstractAnnData.R +++ b/R/AbstractAnnData.R @@ -69,6 +69,10 @@ AbstractAnnData <- R6::R6Class("AbstractAnnData", # nolint #' with all elements having the same number of rows and columns as `var`. varp = function(value) { .abstract_function("ad$varp") + }, + #' @field uns The uns slot. Must be `NULL` or a named list. + uns = function(value) { + .abstract_function("ad$uns") } ), public = list( @@ -253,6 +257,21 @@ AbstractAnnData <- R6::R6Class("AbstractAnnData", # nolint collection }, + # @description `.validate_named_list()` checks for whether a value + # is NULL or a named list and throws an error if it is not. + .validate_named_list = function(collection, label) { + if (is.null(collection)) { + return(collection) + } + + collection_names <- names(collection) + if (!is.list(collection) || ((length(collection) != 0) && is.null(collection_names))) { + stop(paste0(label, " must be a named list, was ", class(collection))) + } + + collection + }, + # @description `.validate_obsvar_dataframe()` checks that the # object is a data.frame and removes explicit dimnames. # @param df A data frame to validate. Should be an obs or a var. diff --git a/R/AnnData.R b/R/AnnData.R index a09fa155..534b6033 100644 --- a/R/AnnData.R +++ b/R/AnnData.R @@ -39,6 +39,8 @@ #' @param varp The varp slot is used to store sparse multi-dimensional #' annotation arrays. It must be either `NULL` or a named list, where each #' element is a sparse matrix where each dimension has length `n_vars`. +#' @param uns The uns slot is used to store unstructured annotation. It must +#' be either `NULL` or a named list. #' #' @export #' @@ -67,7 +69,8 @@ AnnData <- function( obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL) { + varp = NULL, + uns = NULL) { InMemoryAnnData$new( obs_names = obs_names, var_names = var_names, @@ -78,6 +81,7 @@ AnnData <- function( obsm = obsm, varm = varm, obsp = obsp, - varp = varp + varp = varp, + uns = uns ) } diff --git a/R/HDF5AnnData.R b/R/HDF5AnnData.R index 9703716d..d1c5e754 100644 --- a/R/HDF5AnnData.R +++ b/R/HDF5AnnData.R @@ -9,11 +9,7 @@ HDF5AnnData <- R6::R6Class("HDF5AnnData", # nolint .n_obs = NULL, .n_vars = NULL, .obs_names = NULL, - .var_names = NULL, - .obsm = NULL, - .varm = NULL, - .obsp = NULL, - .varp = NULL + .var_names = NULL ), active = list( #' @field X The X slot @@ -183,6 +179,17 @@ HDF5AnnData <- R6::R6Class("HDF5AnnData", # nolint write_h5ad_data_frame_index(value, private$.h5obj, "var", "_index") private$.var_names <- value } + }, + #' @field uns The uns slot. Must be `NULL` or a named list. + uns = function(value) { + if (missing(value)) { + # trackstatus: class=HDF5AnnData, feature=get_uns, status=done + read_h5ad_element(private$.h5obj, "uns") + } else { + # trackstatus: class=HDF5AnnData, feature=set_uns, status=done + value <- private$.validate_named_list(value, "uns") + write_h5ad_element(value, private$.h5obj, "/uns") + } } ), public = list( @@ -221,6 +228,8 @@ HDF5AnnData <- R6::R6Class("HDF5AnnData", # nolint #' @param varp The varp slot is used to store sparse multi-dimensional #' annotation arrays. It must be either `NULL` or a named list, where each #' element is a sparse matrix where each dimension has length `n_vars`. + #' @param uns The uns slot is used to store unstructured annotation. It must + #' be either `NULL` or a named list. #' #' @details #' The constructor creates a new HDF5 AnnData interface object. This can @@ -239,7 +248,8 @@ HDF5AnnData <- R6::R6Class("HDF5AnnData", # nolint obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL) { + varp = NULL, + uns = NULL) { if (!requireNamespace("rhdf5", quietly = TRUE)) { stop("The HDF5 interface requires the 'rhdf5' package to be installed") } @@ -318,6 +328,10 @@ HDF5AnnData <- R6::R6Class("HDF5AnnData", # nolint if (!is.null(varp)) { self$varp <- varp } + + if (!is.null(uns)) { + self$uns <- uns + } }, #' @description Number of observations in the AnnData object @@ -381,6 +395,7 @@ to_HDF5AnnData <- function(adata, file) { # nolint var_names = adata$var_names, layers = adata$layers, obsp = adata$obsp, - varp = adata$varp + varp = adata$varp, + uns = adata$uns ) } diff --git a/R/InMemoryAnnData.R b/R/InMemoryAnnData.R index 47b098e1..b0560604 100644 --- a/R/InMemoryAnnData.R +++ b/R/InMemoryAnnData.R @@ -39,7 +39,8 @@ InMemoryAnnData <- R6::R6Class("InMemoryAnnData", # nolint .obsm = NULL, .varm = NULL, .obsp = NULL, - .varp = NULL + .varp = NULL, + .uns = NULL ), active = list( #' @field X NULL or an observation x variable matrix (without @@ -236,6 +237,8 @@ InMemoryAnnData <- R6::R6Class("InMemoryAnnData", # nolint #' @param varp The varp slot is used to store sparse multi-dimensional #' annotation arrays. It must be either `NULL` or a named list, where each #' element is a sparse matrix where each dimension has length `n_vars`. + #' @param uns The uns slot is used to store unstructured annotation. + #' It must be either `NULL` or a named list. initialize = function(obs_names, var_names, X = NULL, @@ -245,7 +248,8 @@ InMemoryAnnData <- R6::R6Class("InMemoryAnnData", # nolint obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL) { + varp = NULL, + uns = NULL) { # write obs and var first, because these are used by other validators self$obs_names <- obs_names self$var_names <- var_names @@ -259,6 +263,7 @@ InMemoryAnnData <- R6::R6Class("InMemoryAnnData", # nolint self$varm <- varm self$obsp <- obsp self$varp <- varp + self$uns <- uns } ) ) @@ -302,6 +307,7 @@ to_InMemoryAnnData <- function(adata) { # nolint obsm = adata$obsm, varm = adata$varm, obsp = adata$obsp, - varp = adata$varp + varp = adata$varp, + uns = adata$uns ) } From 14d69ede18e9b1657e718acea23d5cf37f7f0d87 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Tue, 19 Sep 2023 15:25:41 +0200 Subject: [PATCH 02/11] update docs --- man/AbstractAnnData.Rd | 2 ++ man/AnnData.Rd | 6 +++++- man/HDF5AnnData.Rd | 8 +++++++- man/InMemoryAnnData.Rd | 6 +++++- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/man/AbstractAnnData.Rd b/man/AbstractAnnData.Rd index 8ac448b8..0dc69536 100644 --- a/man/AbstractAnnData.Rd +++ b/man/AbstractAnnData.Rd @@ -48,6 +48,8 @@ with all elements having the same number of rows and columns as \code{obs}.} \item{\code{varp}}{The varp slot. Must be \code{NULL} or a named list with with all elements having the same number of rows and columns as \code{var}.} + +\item{\code{uns}}{The uns slot. Must be \code{NULL} or a named list.} } \if{html}{\out{}} } diff --git a/man/AnnData.Rd b/man/AnnData.Rd index 3d9ef5da..ba1d44c9 100644 --- a/man/AnnData.Rd +++ b/man/AnnData.Rd @@ -14,7 +14,8 @@ AnnData( obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL + varp = NULL, + uns = NULL ) } \arguments{ @@ -58,6 +59,9 @@ element is a sparse matrix where each dimension has length \code{n_obs}.} \item{varp}{The varp slot is used to store sparse multi-dimensional annotation arrays. It must be either \code{NULL} or a named list, where each element is a sparse matrix where each dimension has length \code{n_vars}.} + +\item{uns}{The uns slot is used to store unstructured annotation. It must +be either \code{NULL} or a named list.} } \description{ This class is used to represent an AnnData object in memory. diff --git a/man/HDF5AnnData.Rd b/man/HDF5AnnData.Rd index 27c57934..7f441c67 100644 --- a/man/HDF5AnnData.Rd +++ b/man/HDF5AnnData.Rd @@ -37,6 +37,8 @@ with all elements having the same number of rows and columns as \code{var}.} \item{\code{obs_names}}{Names of observations} \item{\code{var_names}}{Names of variables} + +\item{\code{uns}}{The uns slot. Must be \code{NULL} or a named list.} } \if{html}{\out{}} } @@ -81,7 +83,8 @@ HDF5AnnData constructor obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL + varp = NULL, + uns = NULL )}\if{html}{\out{}} } @@ -131,6 +134,9 @@ element is a sparse matrix where each dimension has length \code{n_obs}.} \item{\code{varp}}{The varp slot is used to store sparse multi-dimensional annotation arrays. It must be either \code{NULL} or a named list, where each element is a sparse matrix where each dimension has length \code{n_vars}.} + +\item{\code{uns}}{The uns slot is used to store unstructured annotation. It must +be either \code{NULL} or a named list.} } \if{html}{\out{}} } diff --git a/man/InMemoryAnnData.Rd b/man/InMemoryAnnData.Rd index 1a82e68e..c7e1e642 100644 --- a/man/InMemoryAnnData.Rd +++ b/man/InMemoryAnnData.Rd @@ -116,7 +116,8 @@ Inherits from AbstractAnnData. obsm = NULL, varm = NULL, obsp = NULL, - varp = NULL + varp = NULL, + uns = NULL )}\if{html}{\out{}} } @@ -165,6 +166,9 @@ element is a sparse matrix where each dimension has length \code{n_obs}.} \item{\code{varp}}{The varp slot is used to store sparse multi-dimensional annotation arrays. It must be either \code{NULL} or a named list, where each element is a sparse matrix where each dimension has length \code{n_vars}.} + +\item{\code{uns}}{The uns slot is used to store unstructured annotation. +It must be either \code{NULL} or a named list.} } \if{html}{\out{}} } From cfc5a68bc0e92600318085eddfd44dd62977a908 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Tue, 19 Sep 2023 15:30:38 +0200 Subject: [PATCH 03/11] add uns to inmemoryanndata --- R/InMemoryAnnData.R | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/R/InMemoryAnnData.R b/R/InMemoryAnnData.R index b0560604..d424ff51 100644 --- a/R/InMemoryAnnData.R +++ b/R/InMemoryAnnData.R @@ -199,6 +199,17 @@ InMemoryAnnData <- R6::R6Class("InMemoryAnnData", # nolint ) self } + }, + #' @field uns The uns slot. Must be `NULL` or a named list. + uns = function(value) { + if (missing(value)) { + # trackstatus: class=InMemoryAnnData, feature=get_uns, status=done + private$.uns + } else { + # trackstatus: class=InMemoryAnnData, feature=set_uns, status=done + private$.uns <- private$.validate_named_list(value, "uns") + self + } } ), public = list( From 1cb52952449dc643a5b2b233d84b3ed7bb317cae Mon Sep 17 00:00:00 2001 From: Luke Zappia Date: Wed, 29 Nov 2023 14:59:55 +0100 Subject: [PATCH 04/11] Use correct encoding for H5AD numeric scalars --- R/write_h5ad_helpers.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/write_h5ad_helpers.R b/R/write_h5ad_helpers.R index c6b5b24b..6df13e27 100644 --- a/R/write_h5ad_helpers.R +++ b/R/write_h5ad_helpers.R @@ -290,7 +290,7 @@ write_h5ad_numeric_scalar <- function(value, file, name, version = "0.2.0") { rhdf5::h5write(value, file, name) # Write attributes - write_h5ad_encoding(file, name, "numeric", version) + write_h5ad_encoding(file, name, "numeric-scalar", version) } #' Write H5AD mapping From 376f2d7bfc2871b3a052231c55a62d3b258af6a1 Mon Sep 17 00:00:00 2001 From: Luke Zappia Date: Wed, 29 Nov 2023 15:00:29 +0100 Subject: [PATCH 05/11] Actually write file in SCE write_h5ad example --- R/write_h5ad.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/write_h5ad.R b/R/write_h5ad.R index df3e2c98..4067af61 100644 --- a/R/write_h5ad.R +++ b/R/write_h5ad.R @@ -26,7 +26,6 @@ #' #' # Write a SingleCellExperiment as an H5AD #' if (requireNamespace("SingleCellExperiment", quietly = TRUE)) { -#' h5ad_file <- tempfile(fileext = ".h5ad") #' ncells <- 100 #' counts <- matrix(rpois(20000, 5), ncol = ncells) #' logcounts <- log2(counts + 1) @@ -38,11 +37,13 @@ #' assays = list(counts = counts, logcounts = logcounts), #' reducedDims = list(PCA = pca, tSNE = tsne) #' ) +#' +#' h5ad_file <- tempfile(fileext = ".h5ad") +#' write_h5ad(sce, h5ad_file) #' } #' #' # Write a Seurat as a H5AD #' if (requireNamespace("SeuratObject", quietly = TRUE)) { -#' h5ad_file <- tempfile(fileext = ".h5ad") #' counts <- matrix(1:15, 3L, 5L) #' dimnames(counts) <- list( #' letters[1:3], @@ -59,6 +60,7 @@ #' ) #' obj <- SeuratObject::AddMetaData(obj, cell.metadata) #' +#' h5ad_file <- tempfile(fileext = ".h5ad") #' write_h5ad(obj, h5ad_file) #' } write_h5ad <- function(object, path) { From d8246796bc38a469c115a8f93da8b11a07ba1eac Mon Sep 17 00:00:00 2001 From: Luke Zappia Date: Wed, 29 Nov 2023 17:34:56 +0100 Subject: [PATCH 06/11] Fix handling of H5AD Boolean scalars Need to be written as numeric scalars and converted from factors when read --- R/read_h5ad_helpers.R | 9 ++++++++- R/write_h5ad_helpers.R | 15 +++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/R/read_h5ad_helpers.R b/R/read_h5ad_helpers.R index d1151363..4e36922d 100644 --- a/R/read_h5ad_helpers.R +++ b/R/read_h5ad_helpers.R @@ -341,7 +341,14 @@ read_h5ad_string_scalar <- function(file, name, version = "0.2.0") { #' @noRd read_h5ad_numeric_scalar <- function(file, name, version = "0.2.0") { version <- match.arg(version) - rhdf5::h5read(file, name) + scalar <- rhdf5::h5read(file, name) + + # If the numeric vector is Boolean it gets read as a factor by {rhdf5} + if (is.factor(scalar)) { + scalar <- as.logical(scalar) + } + + return(scalar) } #' Read H5AD mapping diff --git a/R/write_h5ad_helpers.R b/R/write_h5ad_helpers.R index 6df13e27..1a3c9c78 100644 --- a/R/write_h5ad_helpers.R +++ b/R/write_h5ad_helpers.R @@ -23,24 +23,24 @@ write_h5ad_element <- function(value, file, name, ...) { # nolint # Sparse matrices if (inherits(value, "sparseMatrix")) { write_fun <- write_h5ad_sparse_array - # Categoricals + # Categoricals } else if (is.factor(value)) { write_fun <- write_h5ad_categorical - # Lists and data frames + # Lists and data frames } else if (is.list(value)) { if (is.data.frame(value)) { write_fun <- write_h5ad_data_frame } else { write_fun <- write_h5ad_mapping } - # Character values + # Character values } else if (is.character(value)) { if (length(value) == 1) { write_fun <- write_h5ad_string_scalar } else { write_fun <- write_h5ad_string_array } - # Numeric values + # Numeric values } else if (is.numeric(value)) { if (length(value) == 1) { write_fun <- write_h5ad_numeric_scalar @@ -49,14 +49,17 @@ write_h5ad_element <- function(value, file, name, ...) { # nolint } else { write_fun <- write_h5ad_dense_array } - # Logical values + # Logical values } else if (is.logical(value)) { if (any(is.na(value))) { write_fun <- write_h5ad_nullable_boolean + } else if (length(value) == 1) { + # Single Booleans should be written as numeric scalars + write_fun <- write_h5ad_numeric_scalar } else { write_fun <- write_h5ad_dense_array } - # Fail if unknown + # Fail if unknown } else { stop("Writing '", class(value), "' objects to H5AD files is not supported") } From 7b2be6e640c7fa8806f6b7e4f09c42ca24035395 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Thu, 30 Nov 2023 18:04:33 +0100 Subject: [PATCH 07/11] apply styler --- R/read_h5ad_helpers.R | 4 ++-- R/write_h5ad_helpers.R | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/R/read_h5ad_helpers.R b/R/read_h5ad_helpers.R index 4e36922d..ed59c136 100644 --- a/R/read_h5ad_helpers.R +++ b/R/read_h5ad_helpers.R @@ -342,12 +342,12 @@ read_h5ad_string_scalar <- function(file, name, version = "0.2.0") { read_h5ad_numeric_scalar <- function(file, name, version = "0.2.0") { version <- match.arg(version) scalar <- rhdf5::h5read(file, name) - + # If the numeric vector is Boolean it gets read as a factor by {rhdf5} if (is.factor(scalar)) { scalar <- as.logical(scalar) } - + return(scalar) } diff --git a/R/write_h5ad_helpers.R b/R/write_h5ad_helpers.R index 1a3c9c78..402f8266 100644 --- a/R/write_h5ad_helpers.R +++ b/R/write_h5ad_helpers.R @@ -23,24 +23,24 @@ write_h5ad_element <- function(value, file, name, ...) { # nolint # Sparse matrices if (inherits(value, "sparseMatrix")) { write_fun <- write_h5ad_sparse_array - # Categoricals + # Categoricals } else if (is.factor(value)) { write_fun <- write_h5ad_categorical - # Lists and data frames + # Lists and data frames } else if (is.list(value)) { if (is.data.frame(value)) { write_fun <- write_h5ad_data_frame } else { write_fun <- write_h5ad_mapping } - # Character values + # Character values } else if (is.character(value)) { if (length(value) == 1) { write_fun <- write_h5ad_string_scalar } else { write_fun <- write_h5ad_string_array } - # Numeric values + # Numeric values } else if (is.numeric(value)) { if (length(value) == 1) { write_fun <- write_h5ad_numeric_scalar @@ -49,7 +49,7 @@ write_h5ad_element <- function(value, file, name, ...) { # nolint } else { write_fun <- write_h5ad_dense_array } - # Logical values + # Logical values } else if (is.logical(value)) { if (any(is.na(value))) { write_fun <- write_h5ad_nullable_boolean @@ -59,7 +59,7 @@ write_h5ad_element <- function(value, file, name, ...) { # nolint } else { write_fun <- write_h5ad_dense_array } - # Fail if unknown + # Fail if unknown } else { stop("Writing '", class(value), "' objects to H5AD files is not supported") } From 4173ad307784345befa76063ad6f0e2ed62928c1 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Sat, 2 Dec 2023 10:39:45 +0100 Subject: [PATCH 08/11] add uns roundtrip test --- tests/testthat/test-roundtrip-uns.R | 83 +++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 tests/testthat/test-roundtrip-uns.R diff --git a/tests/testthat/test-roundtrip-uns.R b/tests/testthat/test-roundtrip-uns.R new file mode 100644 index 00000000..cdf70551 --- /dev/null +++ b/tests/testthat/test-roundtrip-uns.R @@ -0,0 +1,83 @@ +skip_if_no_anndata() +skip_if_not_installed("rhdf5") + +data <- generate_dataset_as_list(10L, 20L) + +uns_names <- names(data$uns) + +for (name in uns_names) { + test_that(paste0("roundtrip with uns '", name, "'"), { + # create anndata + ad <- AnnData( + obs_names = data$obs_names, + var_names = data$var_names, + uns = data$uns[name] + ) + + # write to file + filename <- withr::local_file(paste0("roundtrip_uns_", name, ".h5ad")) + write_h5ad(ad, filename) + + # read from file + ad_new <- read_h5ad(filename, to = "HDF5AnnData") + + # expect slots are unchanged + expect_equal( + ad_new$uns, + data$uns[name], + ignore_attr = TRUE, + tolerance = 1e-6 + ) + }) +} + +for (name in names) { + test_that(paste0("reticulate->hdf5 with uns '", name, "'"), { + # create anndata + ad <- anndata::AnnData( + obs = data.frame(row.names = data$obs_names), + var = data.frame(row.names = data$var_names), + uns = data$uns[name] + ) + + # write to file + filename <- withr::local_file(paste0("reticulate_to_hdf5_uns_", name, ".h5ad")) + ad$write_h5ad(filename) + + # read from file + ad_new <- HDF5AnnData$new(filename) + + # expect slots are unchanged + expect_equal( + ad_new$uns, + data$uns[name], + ignore_attr = TRUE, + tolerance = 1e-6 + ) + }) +} + +for (name in uns_names) { + test_that(paste0("hdf5->reticulate with uns '", name, "'"), { + # write to file + filename <- withr::local_file(paste0("hdf5_to_reticulate_uns_", name, ".h5ad")) + + # make anndata + ad <- HDF5AnnData$new( + file = filename, + obs_names = data$obs_names, + var_names = data$var_names, + uns = data$uns[name] + ) + + # read from file + ad_new <- anndata::read_h5ad(filename) + + expect_equal( + ad_new$uns, + data$uns[name], + ignore_attr = TRUE, + tolerance = 1e-6 + ) + }) +} From 5605659c566f2b662ef0920079a6faca2e2866f0 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Sat, 2 Dec 2023 10:43:45 +0100 Subject: [PATCH 09/11] fix for loop --- tests/testthat/test-roundtrip-uns.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-roundtrip-uns.R b/tests/testthat/test-roundtrip-uns.R index cdf70551..0beaf4f7 100644 --- a/tests/testthat/test-roundtrip-uns.R +++ b/tests/testthat/test-roundtrip-uns.R @@ -31,7 +31,7 @@ for (name in uns_names) { }) } -for (name in names) { +for (name in uns_names) { test_that(paste0("reticulate->hdf5 with uns '", name, "'"), { # create anndata ad <- anndata::AnnData( From 8a0390f14470eaead58945695eb019f194e51cc8 Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Sat, 2 Dec 2023 10:49:58 +0100 Subject: [PATCH 10/11] fix and improve tests --- tests/testthat/test-HDF5-write.R | 26 +++++++++++++------------- tests/testthat/test-roundtrip-uns.R | 12 ++++++++++++ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/tests/testthat/test-HDF5-write.R b/tests/testthat/test-HDF5-write.R index 28ec690c..da13a80d 100644 --- a/tests/testthat/test-HDF5-write.R +++ b/tests/testthat/test-HDF5-write.R @@ -14,7 +14,7 @@ test_that("Writing H5AD dense arrays works", { expect_true(hdf5_path_exists(h5ad_file, "/dense_array")) attrs <- rhdf5::h5readAttributes(h5ad_file, "dense_array") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "array") + expect_equal(attrs[["encoding-type"]], "array") }) test_that("Writing H5AD sparse arrays works", { @@ -28,7 +28,7 @@ test_that("Writing H5AD sparse arrays works", { expect_true(hdf5_path_exists(h5ad_file, "/csc_array/indptr")) attrs <- rhdf5::h5readAttributes(h5ad_file, "csc_array") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "csc_matrix") + expect_equal(attrs[["encoding-type"]], "csc_matrix") csr_array <- as(array, "RsparseMatrix") expect_silent(write_h5ad_element(csr_array, h5ad_file, "csr_array", compression = "none")) @@ -38,7 +38,7 @@ test_that("Writing H5AD sparse arrays works", { expect_true(hdf5_path_exists(h5ad_file, "/csr_array/indptr")) attrs <- rhdf5::h5readAttributes(h5ad_file, "csr_array") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "csr_matrix") + expect_equal(attrs[["encoding-type"]], "csr_matrix") }) test_that("Writing H5AD nullable booleans works", { @@ -49,7 +49,7 @@ test_that("Writing H5AD nullable booleans works", { expect_true(hdf5_path_exists(h5ad_file, "/nullable_bool")) attrs <- rhdf5::h5readAttributes(h5ad_file, "nullable_bool") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "nullable-boolean") + expect_equal(attrs[["encoding-type"]], "nullable-boolean") }) test_that("Writing H5AD nullable integers works", { @@ -60,7 +60,7 @@ test_that("Writing H5AD nullable integers works", { expect_true(hdf5_path_exists(h5ad_file, "/nullable_int")) attrs <- rhdf5::h5readAttributes(h5ad_file, "nullable_int") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "nullable-integer") + expect_equal(attrs[["encoding-type"]], "nullable-integer") }) test_that("Writing H5AD string arrays works", { @@ -70,7 +70,7 @@ test_that("Writing H5AD string arrays works", { expect_true(hdf5_path_exists(h5ad_file, "/string_array")) attrs <- rhdf5::h5readAttributes(h5ad_file, "string_array") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "string-array") + expect_equal(attrs[["encoding-type"]], "string-array") string2d <- matrix(LETTERS[1:20], nrow = 5, ncol = 4) @@ -78,7 +78,7 @@ test_that("Writing H5AD string arrays works", { expect_true(hdf5_path_exists(h5ad_file, "/string_array2D")) attrs <- rhdf5::h5readAttributes(h5ad_file, "string_array2D") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "string-array") + expect_equal(attrs[["encoding-type"]], "string-array") }) # TODO: re-enable @@ -93,7 +93,7 @@ test_that("Writing H5AD string arrays works", { # expect_true(hdf5_path_exists(h5ad_file, "/categorical/ordered")) # attrs <- rhdf5::h5readAttributes(h5ad_file, "categorical") # expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) -# expect_true(attrs[["encoding-type"]] == "categorical") +# expect_equal(attrs[["encoding-type"]], "categorical") # }) # nolint end @@ -104,7 +104,7 @@ test_that("Writing H5AD string scalars works", { expect_true(hdf5_path_exists(h5ad_file, "/string_scalar")) attrs <- rhdf5::h5readAttributes(h5ad_file, "string_scalar") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "string") + expect_equal(attrs[["encoding-type"]], "string") }) test_that("Writing H5AD numeric scalars works", { @@ -114,7 +114,7 @@ test_that("Writing H5AD numeric scalars works", { expect_true(hdf5_path_exists(h5ad_file, "/numeric_scalar")) attrs <- rhdf5::h5readAttributes(h5ad_file, "numeric_scalar") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "numeric") + expect_equal(attrs[["encoding-type"]], "numeric-scalar") }) test_that("Writing H5AD mappings works", { @@ -138,7 +138,7 @@ test_that("Writing H5AD mappings works", { expect_true(hdf5_path_exists(h5ad_file, "/mapping/scalar")) attrs <- rhdf5::h5readAttributes(h5ad_file, "mapping") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "dict") + expect_equal(attrs[["encoding-type"]], "dict") }) test_that("Writing H5AD data frames works", { @@ -154,9 +154,9 @@ test_that("Writing H5AD data frames works", { expect_true(hdf5_path_exists(h5ad_file, "/dataframe/_index")) attrs <- rhdf5::h5readAttributes(h5ad_file, "dataframe") expect_true(all(c("encoding-type", "encoding-version") %in% names(attrs))) - expect_true(attrs[["encoding-type"]] == "dataframe") + expect_equal(attrs[["encoding-type"]], "dataframe") expect_true(all(c("_index", "column-order") %in% names(attrs))) - expect_true(attrs[["_index"]] == "_index") + expect_equal(attrs[["_index"]], "_index") expect_identical(as.vector(attrs[["column-order"]]), c("Letters", "Numbers")) }) diff --git a/tests/testthat/test-roundtrip-uns.R b/tests/testthat/test-roundtrip-uns.R index 0beaf4f7..424d919d 100644 --- a/tests/testthat/test-roundtrip-uns.R +++ b/tests/testthat/test-roundtrip-uns.R @@ -4,6 +4,18 @@ skip_if_not_installed("rhdf5") data <- generate_dataset_as_list(10L, 20L) uns_names <- names(data$uns) +# TODO: re-enable these tests +uns_names <- uns_names[!grepl("_with_nas", uns_names)] +# TODO: re-enable these tests +uns_names <- uns_names[!grepl("_na$", uns_names)] +# TODO: re-enable these tests +uns_names <- uns_names[!grepl("obsm_", uns_names)] +# TODO: re-enable these tests +uns_names <- uns_names[!uns_names %in% c("vec_factor", "vec_factor_ordered", "vec_logical")] +# TODO: re-enable these tests +uns_names <- uns_names[uns_names != "list"] +# TODO: re-enable these tests +uns_names <- uns_names[!uns_names %in% c("logical", "factor")] for (name in uns_names) { test_that(paste0("roundtrip with uns '", name, "'"), { From 91fb11c81cade62e6db5f7c09ac0e2b9d160326c Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Sat, 2 Dec 2023 10:53:30 +0100 Subject: [PATCH 11/11] fix incorrect merging of main branch with d8246796bc38a469c115a8f93da8b11a07ba1eac --- R/write_h5ad_helpers.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/write_h5ad_helpers.R b/R/write_h5ad_helpers.R index 153640ad..d61c210e 100644 --- a/R/write_h5ad_helpers.R +++ b/R/write_h5ad_helpers.R @@ -53,6 +53,9 @@ write_h5ad_element <- function(value, file, name, compression = c("none", "gzip" } else if (is.logical(value)) { # Logical values if (any(is.na(value))) { write_h5ad_nullable_boolean + } else if (length(value) == 1) { + # Single Booleans should be written as numeric scalars + write_h5ad_numeric_scalar } else { write_h5ad_dense_array }