From a0624f8ca273795bb7a09128c303ee7f72e9da1c Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Wed, 3 Apr 2024 16:01:33 -0400 Subject: [PATCH] [r] Make `TileDBArray$reopen()` public Expose `TileDBArray$reopen()` as a public API; as R does not have context managers, there's no native way to do the Python construct ```python cls = type(arr) with cls.open(arr.uri, "r") as readarr: # do a read operation even though the array is open in write ``` This PR exposes `$reopen()` to reopen an array in a new mode Modified SOMA methods: - `TileDBArray$reopen()`: now public and includes new default for reopening: if `arr$mode()` is `READ`, will automatically reopen in `WRITE` and vice versa - `TileDBObject$is_open()`: now uses `TileDBObject$mode()` to determine if the array is open as `TileDBObject$mode()` already processes `TileDBObject$private$.mode` --- apis/r/R/SOMADataFrame.R | 4 +- apis/r/R/TileDBArray.R | 45 ++++++++--- apis/r/R/TileDBObject.R | 2 +- apis/r/man/SOMAArrayBase.Rd | 1 + apis/r/man/SOMADataFrame.Rd | 1 + apis/r/man/SOMADenseNDArray.Rd | 1 + apis/r/man/SOMANDArrayBase.Rd | 1 + apis/r/man/SOMASparseNDArray.Rd | 1 + apis/r/man/TileDBArray.Rd | 26 ++++++ apis/r/tests/testthat/test-SeuratIngest.R | 6 +- apis/r/tests/testthat/test-reopen.R | 99 +++++++++++++++++++++++ 11 files changed, 169 insertions(+), 18 deletions(-) create mode 100644 apis/r/tests/testthat/test-reopen.R diff --git a/apis/r/R/SOMADataFrame.R b/apis/r/R/SOMADataFrame.R index f05912d7cb..df0d71e026 100644 --- a/apis/r/R/SOMADataFrame.R +++ b/apis/r/R/SOMADataFrame.R @@ -314,7 +314,7 @@ SOMADataFrame <- R6::R6Class( # - validate number of rows in values matches number of rows in array # - add original soma_joinids to values if not present spdl::debug("[SOMADataFrame update]: Retrieving existing soma_joinids") - private$reopen(mode = "READ") + self$reopen(mode = "READ") joinids <- self$read(column_names = "soma_joinid")$concat()$soma_joinid if (length(joinids) != nrow(values)) { stop( @@ -392,7 +392,7 @@ SOMADataFrame <- R6::R6Class( se <- tiledb::tiledb_array_schema_evolution_array_evolve(se, self$uri) # Reopen array for writing with new schema - private$reopen(mode = "WRITE") + self$reopen(mode = "WRITE") spdl::info("[SOMADataFrame update]: Writing new data") self$write(values) } diff --git a/apis/r/R/TileDBArray.R b/apis/r/R/TileDBArray.R index ae230e0089..6fa8d6a861 100644 --- a/apis/r/R/TileDBArray.R +++ b/apis/r/R/TileDBArray.R @@ -49,6 +49,29 @@ TileDBArray <- R6::R6Class( invisible(self) }, + #' @description Close and reopen the TileDB object in a new mode + #' + #' @param mode New mode to open the object in; choose from: + #' \itemize{ + #' \item \dQuote{\code{READ}} + #' \item \dQuote{\code{WRITE}} + #' } + #' By default, reopens in the opposite mode of the current mode + #' + #' @return Invisibly returns \code{self} + #' + reopen = function(mode = NULL) { + modes <- c(READ = 'WRITE', WRITE = 'READ') + oldmode <- self$mode() + mode <- mode %||% modes[oldmode] + mode <- match.arg(mode, choices = modes) + if (mode != oldmode) { + self$close() + self$open(mode, internal_use_only = 'allowed_use') + } + return(invisible(self)) + }, + #' @description Print summary of the array. (lifecycle: experimental) print = function() { super$print() @@ -289,17 +312,17 @@ TileDBArray <- R6::R6Class( private = list( - # Reopen the array in a different mode - reopen = function(mode = c("READ", "WRITE")) { - mode <- match.arg(mode) - if (private$.mode == mode) { - return(invisible(self)) - } - self$close() - invisible( - self$open(mode = mode, internal_use_only = "allowed_use") - ) - }, + # # Reopen the array in a different mode + # reopen = function(mode = c("READ", "WRITE")) { + # mode <- match.arg(mode) + # if (private$.mode == mode) { + # return(invisible(self)) + # } + # self$close() + # invisible( + # self$open(mode = mode, internal_use_only = "allowed_use") + # ) + # }, # Internal pointer to the TileDB array. # diff --git a/apis/r/R/TileDBObject.R b/apis/r/R/TileDBObject.R index 7907d38571..2dc555a020 100644 --- a/apis/r/R/TileDBObject.R +++ b/apis/r/R/TileDBObject.R @@ -58,7 +58,7 @@ TileDBObject <- R6::R6Class( #' @return \code{TRUE} if the object is open, otherwise \code{FALSE} #' is_open = function() { - !is.null(private$.mode) + return(self$mode() != 'CLOSED') }, # TODO: make this an active diff --git a/apis/r/man/SOMAArrayBase.Rd b/apis/r/man/SOMAArrayBase.Rd index d78aea0488..44e1278a6b 100644 --- a/apis/r/man/SOMAArrayBase.Rd +++ b/apis/r/man/SOMAArrayBase.Rd @@ -45,6 +45,7 @@ experimental)
  • tiledbsoma::TileDBArray$non_empty_domain()
  • tiledbsoma::TileDBArray$open()
  • tiledbsoma::TileDBArray$print()
    • +
  • tiledbsoma::TileDBArray$reopen()
  • tiledbsoma::TileDBArray$schema()
  • tiledbsoma::TileDBArray$set_metadata()
  • tiledbsoma::TileDBArray$shape()
    • diff --git a/apis/r/man/SOMADataFrame.Rd b/apis/r/man/SOMADataFrame.Rd index 6f5b4003d8..7e3b7d93b5 100644 --- a/apis/r/man/SOMADataFrame.Rd +++ b/apis/r/man/SOMADataFrame.Rd @@ -43,6 +43,7 @@ row and is intended to act as a join key for other objects, such as
  • tiledbsoma::TileDBArray$non_empty_domain()
  • tiledbsoma::TileDBArray$open()
  • tiledbsoma::TileDBArray$print()
    • +
  • tiledbsoma::TileDBArray$reopen()
  • tiledbsoma::TileDBArray$schema()
  • tiledbsoma::TileDBArray$set_metadata()
  • tiledbsoma::TileDBArray$shape()
    • diff --git a/apis/r/man/SOMADenseNDArray.Rd b/apis/r/man/SOMADenseNDArray.Rd index ae6bf3bc8c..a2c1c459a3 100644 --- a/apis/r/man/SOMADenseNDArray.Rd +++ b/apis/r/man/SOMADenseNDArray.Rd @@ -57,6 +57,7 @@ The \code{write} method is currently limited to writing from 2-d matrices.
  • tiledbsoma::TileDBArray$non_empty_domain()
  • tiledbsoma::TileDBArray$open()
  • tiledbsoma::TileDBArray$print()
    • +
  • tiledbsoma::TileDBArray$reopen()
  • tiledbsoma::TileDBArray$schema()
  • tiledbsoma::TileDBArray$set_metadata()
  • tiledbsoma::TileDBArray$shape()
    • diff --git a/apis/r/man/SOMANDArrayBase.Rd b/apis/r/man/SOMANDArrayBase.Rd index 1fcd9365da..475c38aae7 100644 --- a/apis/r/man/SOMANDArrayBase.Rd +++ b/apis/r/man/SOMANDArrayBase.Rd @@ -39,6 +39,7 @@ Adds NDArray-specific functionality to the \code{\link{SOMAArrayBase}} class.
  • tiledbsoma::TileDBArray$non_empty_domain()
  • tiledbsoma::TileDBArray$open()
  • tiledbsoma::TileDBArray$print()
    • +
  • tiledbsoma::TileDBArray$reopen()
  • tiledbsoma::TileDBArray$schema()
  • tiledbsoma::TileDBArray$set_metadata()
  • tiledbsoma::TileDBArray$shape()
    • diff --git a/apis/r/man/SOMASparseNDArray.Rd b/apis/r/man/SOMASparseNDArray.Rd index 3d80042667..1359cac91f 100644 --- a/apis/r/man/SOMASparseNDArray.Rd +++ b/apis/r/man/SOMASparseNDArray.Rd @@ -55,6 +55,7 @@ the object are overwritten and new index values are added. (lifecycle: experimen
  • tiledbsoma::TileDBArray$non_empty_domain()
  • tiledbsoma::TileDBArray$open()
  • tiledbsoma::TileDBArray$print()
    • +
  • tiledbsoma::TileDBArray$reopen()
  • tiledbsoma::TileDBArray$schema()
  • tiledbsoma::TileDBArray$set_metadata()
  • tiledbsoma::TileDBArray$shape()
    • diff --git a/apis/r/man/TileDBArray.Rd b/apis/r/man/TileDBArray.Rd index ffaebdcbe5..8e0d12e137 100644 --- a/apis/r/man/TileDBArray.Rd +++ b/apis/r/man/TileDBArray.Rd @@ -24,6 +24,7 @@ Base class for representing an individual TileDB array. \itemize{ \item \href{#method-TileDBArray-open}{\code{TileDBArray$open()}} \item \href{#method-TileDBArray-close}{\code{TileDBArray$close()}} +\item \href{#method-TileDBArray-reopen}{\code{TileDBArray$reopen()}} \item \href{#method-TileDBArray-print}{\code{TileDBArray$print()}} \item \href{#method-TileDBArray-tiledb_array}{\code{TileDBArray$tiledb_array()}} \item \href{#method-TileDBArray-get_metadata}{\code{TileDBArray$get_metadata()}} @@ -92,6 +93,31 @@ The object, invisibly } } \if{html}{\out{
    }} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-TileDBArray-reopen}{}}} +\subsection{Method \code{reopen()}}{ +Close and reopen the TileDB object in a new mode +\subsection{Usage}{ +\if{html}{\out{
    }}\preformatted{TileDBArray$reopen(mode = NULL)}\if{html}{\out{
    }} +} + +\subsection{Arguments}{ +\if{html}{\out{
    }} +\describe{ +\item{\code{mode}}{New mode to open the object in; choose from: +\itemize{ +\item \dQuote{\code{READ}} +\item \dQuote{\code{WRITE}} +} +By default, reopens in the opposite mode of the current mode} +} +\if{html}{\out{
    }} +} +\subsection{Returns}{ +Invisibly returns \code{self} +} +} +\if{html}{\out{
    }} \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-TileDBArray-print}{}}} \subsection{Method \code{print()}}{ diff --git a/apis/r/tests/testthat/test-SeuratIngest.R b/apis/r/tests/testthat/test-SeuratIngest.R index 8ba13f9879..1179782ca5 100644 --- a/apis/r/tests/testthat/test-SeuratIngest.R +++ b/apis/r/tests/testthat/test-SeuratIngest.R @@ -203,7 +203,7 @@ test_that("Write SeuratCommand mechanics", { uri <- withr::local_tempdir('write-command-log') uns <- SOMACollectionCreate(uri) - on.exit(uns$close, add = TRUE) + on.exit(uns$close(), add = TRUE, after = FALSE) pbmc_small <- get_data('pbmc_small', package = 'SeuratObject') for (cmd in SeuratObject::Command(pbmc_small)) { @@ -214,9 +214,7 @@ test_that("Write SeuratCommand mechanics", { expect_s3_class(cmdgrp <- uns$get('seurat_commands'), 'SOMACollection') expect_s3_class(cmddf <- cmdgrp$get(cmd), 'SOMADataFrame') - expect_identical(cmddf$mode(), 'CLOSED') - - expect_no_condition(cmddf <- cmddf$open('READ', internal_use_only = 'allowed_use')) + expect_invisible(cmddf$reopen("READ")) on.exit(cmddf$close(), add = TRUE, after = FALSE) # Test qualities of the SOMADataFrame diff --git a/apis/r/tests/testthat/test-reopen.R b/apis/r/tests/testthat/test-reopen.R new file mode 100644 index 0000000000..228bbccb4f --- /dev/null +++ b/apis/r/tests/testthat/test-reopen.R @@ -0,0 +1,99 @@ +test_that("Test reopen works on arrays", { + shape <- c(500L, 100L) + for (cls in c("SOMADataFrame", "SOMASparseNDArray", "SOMADenseNDArray")) { + uri <- withr::local_tempdir(paste("soma", cls, "array", "reopen", sep = "-")) + arr <- switch( + EXPR = cls, + SOMADataFrame = SOMADataFrameCreate( + uri, + schema = arrow::infer_schema(data.frame( + soma_joinid = bit64::integer64(), + int = integer() + )) + ), + SOMASparseNDArray = SOMASparseNDArrayCreate( + uri, + type = arrow::int32(), + shape = shape + ), + SOMADenseNDArray = SOMADenseNDArrayCreate( + uri, + type = arrow::int32(), + shape = shape + ) + ) + expect_s3_class(arr, cls) + expect_identical( + arr$mode(), + "WRITE", + info = sprintf("%sCreate() returns object open for 'WRITE'", cls) + ) + expect_true( + arr$is_open(), + info = sprintf("%s is open when mode is 'WRITE'", cls) + ) + + lab <- sprintf("%s$reopen()", cls) + is_open <- sprintf("%s$reopen() returns an object object", cls) + + # Test implicit WRITE -> READ + expect_invisible(arr$reopen(), label = lab) + expect_identical( + arr$mode(), + "READ", + info = sprintf("%s$reopen() when mode is 'WRITE' reopens as 'READ'", cls) + ) + expect_true(arr$is_open(), info = is_open) + + # Test implicit READ -> WRITE + expect_invisible(arr$reopen(), label = lab) + expect_identical( + arr$mode(), + "WRITE", + info = sprintf("%s$reopen() when mode is 'READ' reopens as 'WRITE'", cls) + ) + expect_true(arr$is_open(), info = is_open) + + # Test reopening in the same mode + orig <- arr$mode() + expect_invisible(arr$reopen(orig), label = lab) + expect_identical( + arr$mode(), + orig, + info = sprintf("%s$reopen() with an identical mode returns the same mode", cls) + ) + expect_true(arr$is_open(), info = is_open) + + # Test reopen from close + expect_no_condition(arr$close()) + expect_identical(arr$mode(), "CLOSED") + expect_false(arr$is_open()) + + for (mode in c("READ", "WRITE")) { + expect_invisible( + arr$reopen(mode), + label = sprintf("%s$reopen('%s') from closed", cls, mode) + ) + expect_identical( + arr$mode(), + mode, + info = sprintf("%s$reopen('%s') returns a mode of '%s'", cls, mode, mode) + ) + expect_true( + arr$is_open(), + info = sprintf("%s$reopen('%s') from CLOSED returns an open object", cls, mode) + ) + expect_no_condition(arr$close()) + } + + arr$close() + expect_error(arr$reopen()) + + # Test assertions + expect_error(arr$reopen("tomato")) + expect_error(arr$reopen(TRUE)) + expect_error(arr$reopen(1L)) + + arr$close() + } +})