Repository: single-cell-data/TileDB-SOMA (language: python)

Issue:
[r] Timestamp write and read of data frame and array objects
**Issue and/or context:**

This PR adds the ability to write, and read, at given time stamps covering the key SOMA object data frame as well as sparse and dense arrays.

There is a corresponding follow-up PR #2925 which exposes time points (as before) as well as range (begin and end) as taken by the C++ API.

**Changes:**

Code changes as well as a new test file.  It also contains two additional smaller PRs (#2866, #2871).

**Notes for Reviewer:**

[SC 52516](https://app.shortcut.com/tiledb-inc/story/52516/r-support-timestamp-range)  

The branch had been open for two-plus weeks and has been rebased a few times so the final commit timestamps are not from the initial commits.


Discussion / hints:
## [Codecov](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 89.88%. Comparing base [(`331a1b6`)](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/commit/331a1b62a0c72fd6f58cfebf97a80a02fd4aa74f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) to head [(`af225dc`)](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/commit/af225dcf534a67c279bc7573746d9ed61df70517?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data).
> Report is 2 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #2926   +/-   ##
=======================================
  Coverage   89.88%   89.88%           
=======================================
  Files          37       37           
  Lines        3925     3925           
=======================================
  Hits         3528     3528           
  Misses        397      397           
```

| [Flag](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) | Coverage Δ | |
|---|---|---|
| [python](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) | `89.88% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Components](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) | Coverage Δ | |
|---|---|---|
| [python_api](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) | `89.88% <ø> (ø)` | |
| [libtiledbsoma](https://app.codecov.io/gh/single-cell-data/TileDB-SOMA/pull/2926/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=single-cell-data) | `∅ <ø> (∅)` | |

</details>
I put one microfix in following review but as discussed in parallel it maybe beneficial to attach this in parallel.
Thanks for #2929 and the follow-up comments here.

Fix patch:
diff --git a/apis/r/DESCRIPTION b/apis/r/DESCRIPTION
index 5ada792d2b..72dcbf2b0c 100644
--- a/apis/r/DESCRIPTION
+++ b/apis/r/DESCRIPTION
@@ -6,7 +6,7 @@ Description: Interface for working with 'TileDB'-based Stack of Matrices,
     like those commonly used for single cell data analysis. It is documented at
     <https://github.com/single-cell-data>; a formal specification available is at
     <https://github.com/single-cell-data/SOMA/blob/main/abstract_specification.md>.
-Version: 1.13.99.4
+Version: 1.13.99.5
 Authors@R: c(
     person(given = "Aaron", family = "Wolen",
            role = c("cre", "aut"), email = "aaron@tiledb.com",
diff --git a/apis/r/NEWS.md b/apis/r/NEWS.md
index aa61d10905..4707c5fcc3 100644
--- a/apis/r/NEWS.md
+++ b/apis/r/NEWS.md
@@ -4,6 +4,7 @@
 
 * Make use of timestamp ranges in libtiledbsoma
 * Simplify timestamp ranges; strengthen assumptions about `tiledb_timestamp`
+* Use cached timestamps in `$write()` and `$create()`
 
 # tiledbsoma 1.13.0
 
diff --git a/apis/r/R/Factory.R b/apis/r/R/Factory.R
index 17eff71158..fdb6c2ed1b 100644
--- a/apis/r/R/Factory.R
+++ b/apis/r/R/Factory.R
@@ -49,7 +49,6 @@ SOMADataFrameCreate <- function(
       schema,
       index_column_names = index_column_names,
       platform_config = platform_config,
-      timestamps = rep(tiledb_timestamp, 2),
       internal_use_only = "allowed_use"
     )
   }
@@ -76,7 +75,7 @@ SOMADataFrameOpen <- function(
   tiledbsoma_ctx = NULL,
   tiledb_timestamp = NULL
 ) {
-  spdl::debug("[SOMADataFrameOpen] uri {} ts ({},{})", uri, tiledb_timestamp[1], tiledb_timestamp[2])
+  spdl::debug("[SOMADataFrameOpen] uri {} ts ({})", uri, tiledb_timestamp %||% "now")
   sdf <- SOMADataFrame$new(
     uri,
     platform_config,
@@ -127,7 +126,6 @@ SOMASparseNDArrayCreate <- function(
       type,
       shape,
       platform_config = platform_config,
-      timestamps = rep(tiledb_timestamp, 2),
       internal_use_only = "allowed_use"
     )
   }
@@ -176,7 +174,7 @@ SOMADenseNDArrayCreate <- function(
   tiledbsoma_ctx = NULL,
   tiledb_timestamp = NULL
 ) {
-  spdl::debug("[SOMADenseNDArrayCreate] tstamp ({},{})", tiledb_timestamp[1], tiledb_timestamp[2])
+  spdl::debug("[SOMADenseNDArrayCreate] tstamp ({})", tiledb_timestamp %||% "now")
   dnda <- SOMADenseNDArray$new(
     uri,
     platform_config,
@@ -188,7 +186,6 @@ SOMADenseNDArrayCreate <- function(
     type,
     shape,
     platform_config = platform_config,
-    timestamps = rep(tiledb_timestamp, 2),
     internal_use_only = "allowed_use"
   )
   return(dnda)
diff --git a/apis/r/R/SOMAArrayBase.R b/apis/r/R/SOMAArrayBase.R
index 06b7a2e8ff..4d9082c0f5 100644
--- a/apis/r/R/SOMAArrayBase.R
+++ b/apis/r/R/SOMAArrayBase.R
@@ -29,14 +29,14 @@ SOMAArrayBase <- R6::R6Class(
       private$soma_type_cache <- self$get_metadata(SOMA_OBJECT_TYPE_METADATA_KEY)
     },
 
-    write_object_type_metadata = function(timestamps=NULL) {
+    write_object_type_metadata = function() {
       #private$check_open_for_write()
 
       meta <- list()
       meta[[SOMA_OBJECT_TYPE_METADATA_KEY]] <- self$class()
       meta[[SOMA_ENCODING_VERSION_METADATA_KEY]] <- SOMA_ENCODING_VERSION
       spdl::debug("[SOMAArrayBase::write_object_metadata] calling set metadata")
-      self$set_metadata(meta, self$.tiledb_timestamp_range)
+      self$set_metadata(meta)
     }
   )
 )
diff --git a/apis/r/R/SOMADataFrame.R b/apis/r/R/SOMADataFrame.R
index b27c852e79..62877e6a5f 100644
--- a/apis/r/R/SOMADataFrame.R
+++ b/apis/r/R/SOMADataFrame.R
@@ -21,11 +21,14 @@ SOMADataFrame <- R6::R6Class(
     #' index columns.  All named columns must exist in the schema, and at least
     #' one index column name is required.
     #' @template param-platform-config
-    #' @param timestamps Optional timestamp start and end range
     #' @param internal_use_only Character value to signal this is a 'permitted' call,
     #' as `create()` is considered internal and should not be called directly.
-    create = function(schema, index_column_names = c("soma_joinid"),
-                      platform_config = NULL, timestamps = NULL, internal_use_only = NULL) {
+    create = function(
+      schema,
+      index_column_names = c("soma_joinid"),
+      platform_config = NULL,
+      internal_use_only = NULL
+    ) {
       if (is.null(internal_use_only) || internal_use_only != "allowed_use") {
         stop(paste("Use of the create() method is for internal use only. Consider using a",
                    "factory method as e.g. 'SOMADataFrameCreate()'."), call. = FALSE)
@@ -43,7 +46,11 @@ SOMADataFrame <- R6::R6Class(
       ## we (currently pass domain and extent values in an arrow table (i.e. data.frame alike)
       ## where each dimension is one column (of the same type as in the schema) followed by three
       ## values for the domain pair and the extent
-      dom_ext_tbl <- get_domain_and_extent_dataframe(schema, index_column_names, tiledb_create_options)
+      dom_ext_tbl <- get_domain_and_extent_dataframe(
+        schema,
+        ind_col_names = index_column_names,
+        tdco = tiledb_create_options
+      )
 
       ## we transfer to the arrow table via a pair of array and schema pointers
       dnaap <- nanoarrow::nanoarrow_allocate_array()
@@ -56,11 +63,20 @@ SOMADataFrame <- R6::R6Class(
 
       spdl::debug("[SOMADataFrame$create] about to create schema from arrow")
       ctxptr <- super$tiledbsoma_ctx$context()
-      createSchemaFromArrow(uri = self$uri, nasp, dnaap, dnasp, TRUE, "SOMADataFrame",
-                            tiledb_create_options$to_list(FALSE), soma_context(), timestamps)
+      createSchemaFromArrow(
+        uri = self$uri,
+        nasp = nasp,
+        nadimap = dnaap,
+        nadimsp = dnasp,
+        sparse = TRUE,
+        datatype = "SOMADataFrame",
+        pclst = tiledb_create_options$to_list(FALSE),
+        ctxxp = soma_context(),
+        tsvec = self$.tiledb_timestamp_range
+      )
 
       spdl::debug("[SOMADataFrame$create] about to call write_object_type_metadata")
-      private$write_object_type_metadata(timestamps)
+      private$write_object_type_metadata()
 
       self$open("WRITE", internal_use_only = "allowed_use")
       self
@@ -71,10 +87,8 @@ SOMADataFrame <- R6::R6Class(
     #' @param values An [`arrow::Table`] or [`arrow::RecordBatch`]
     #' containing all columns, including any index columns. The
     #' schema for `values` must match the schema for the `SOMADataFrame`.
-    #' @param tsrange An optional two-element Datetime vector for the
-    #' start and end of the timestamp range
     #'
-    write = function(values, tsrange = NULL) {
+    write = function(values) {
       private$check_open_for_write()
 
       # Prevent downcasting of int64 to int32 when materializing a column
@@ -103,7 +117,14 @@ SOMADataFrame <- R6::R6Class(
 
       df <- as.data.frame(values)[schema_names]
       arr <- self$object
-      writeArrayFromArrow(self$uri, naap, nasp, "SOMADataFrame", NULL, tsrange)
+      writeArrayFromArrow(
+        uri = self$uri,
+        naap = naap,
+        nasp = nasp,
+        arraytype = "SOMADataFrame",
+        config = NULL,
+        tsvec = self$.tiledb_timestamp_range
+      )
 
       invisible(self)
     },
diff --git a/apis/r/R/SOMANDArrayBase.R b/apis/r/R/SOMANDArrayBase.R
index 25a11ed73e..a52fa13235 100644
--- a/apis/r/R/SOMANDArrayBase.R
+++ b/apis/r/R/SOMANDArrayBase.R
@@ -20,12 +20,15 @@ SOMANDArrayBase <- R6::R6Class(
     #' element in the array.
     #' @param shape a vector of integers defining the shape of the array.
     #' @template param-platform-config
-    #' @param timestamps Optional timestamp start and end range
     #' @param internal_use_only Character value to signal this is a 'permitted'
     #' call, as `create()` is considered internal and should not be called
     #' directly.
-    create = function(type, shape, platform_config = NULL,
-                      timestamps = NULL, internal_use_only = NULL) {
+    create = function(
+      type,
+      shape,
+      platform_config = NULL,
+      internal_use_only = NULL
+    ) {
       if (is.null(internal_use_only) || internal_use_only != "allowed_use") {
         stop(paste("Use of the create() method is for internal use only. Consider using a",
                    "factory method as e.g. 'SOMASparseNDArrayCreate()'."), call. = FALSE)
@@ -58,10 +61,17 @@ SOMANDArrayBase <- R6::R6Class(
 
       ## create array
       ctxptr <- super$tiledbsoma_ctx$context()
-      createSchemaFromArrow(uri = self$uri, nasp, dnaap, dnasp,
-                            private$.is_sparse,
-                            if (private$.is_sparse) "SOMASparseNDArray" else "SOMADenseNDArray",
-                            tiledb_create_options$to_list(FALSE), soma_context(), timestamps)
+      createSchemaFromArrow(
+        uri = self$uri,
+        nasp = nasp,
+        nadimap = dnaap,
+        nadimsp = dnasp,
+        sparse = private$.is_sparse,
+        datatype = if (private$.is_sparse) "SOMASparseNDArray" else "SOMADenseNDArray",
+        pclst = tiledb_create_options$to_list(FALSE),
+        ctxxp = soma_context(),
+        tsvec = self$.tiledb_timestamp_range
+      )
       #private$write_object_type_metadata(timestamps)  ## FIXME: temp. commented out -- can this be removed overall?
 
       self$open("WRITE", internal_use_only = "allowed_use")
diff --git a/apis/r/R/SOMASparseNDArray.R b/apis/r/R/SOMASparseNDArray.R
index 3e91ea32f1..1144780cde 100644
--- a/apis/r/R/SOMASparseNDArray.R
+++ b/apis/r/R/SOMASparseNDArray.R
@@ -174,7 +174,7 @@ SOMASparseNDArray <- R6::R6Class(
         names(bbox_flat)[index:(index + 1L)] <- paste0(names(bbox)[i], c('_lower', '_upper'))
         index <- index + 2L
       }
-      self$set_metadata(bbox_flat, self$.tiledb_timestamp_range)
+      self$set_metadata(bbox_flat)
       spdl::debug(
         "[SOMASparseNDArray$write] Calling .write_coo_df ({})",
         self$tiledb_timestamp %||% "now"
diff --git a/apis/r/R/TileDBArray.R b/apis/r/R/TileDBArray.R
index 22d9a01d54..9ebba1a873 100644
--- a/apis/r/R/TileDBArray.R
+++ b/apis/r/R/TileDBArray.R
@@ -97,9 +97,8 @@ TileDBArray <- R6::R6Class(
 
     #' @description Add list of metadata to the specified TileDB array. (lifecycle: maturing)
     #' @param metadata Named list of metadata to add.
-    #' @param timestamps Optional two-element vector of timestamp range starts and end
     #' @return NULL
-    set_metadata = function(metadata, timestamps = NULL) {
+    set_metadata = function(metadata) {
       stopifnot(
         "Metadata must be a named list" = is_named_list(metadata)
       )
@@ -109,7 +108,15 @@ TileDBArray <- R6::R6Class(
       for (nm in names(metadata)) {
           val <- metadata[[nm]]
           spdl::debug("[TileDBArray$set_metadata] setting key {} to {} ({})", nm, val, class(val))
-          set_metadata(self$uri, nm, val, class(val), TRUE, soma_context(), timestamps)
+          set_metadata(
+            uri = self$uri,
+            key = nm,
+            valuesxp = val,
+            type = class(val),
+            is_array = TRUE,
+            ctxxp = soma_context(),
+            tsvec = self$.tiledb_timestamp_range
+          )
       }
 
       dev_null <- mapply(
diff --git a/apis/r/man/SOMADataFrame.Rd b/apis/r/man/SOMADataFrame.Rd
index 9c3a2d550b..1e84308920 100644
--- a/apis/r/man/SOMADataFrame.Rd
+++ b/apis/r/man/SOMADataFrame.Rd
@@ -63,7 +63,6 @@ Create (lifecycle: maturing)
   schema,
   index_column_names = c("soma_joinid"),
   platform_config = NULL,
-  timestamps = NULL,
   internal_use_only = NULL
 )}\if{html}{\out{</div>}}
 }
@@ -80,8 +79,6 @@ one index column name is required.}
 \item{\code{platform_config}}{A \link[tiledbsoma:PlatformConfig]{platform configuration}
 object}
 
-\item{\code{timestamps}}{Optional timestamp start and end range}
-
 \item{\code{internal_use_only}}{Character value to signal this is a 'permitted' call,
 as \code{create()} is considered internal and should not be called directly.}
 }
@@ -94,7 +91,7 @@ as \code{create()} is considered internal and should not be called directly.}
 \subsection{Method \code{write()}}{
 Write (lifecycle: maturing)
 \subsection{Usage}{
-\if{html}{\out{<div class="r">}}\preformatted{SOMADataFrame$write(values, tsrange = NULL)}\if{html}{\out{</div>}}
+\if{html}{\out{<div class="r">}}\preformatted{SOMADataFrame$write(values)}\if{html}{\out{</div>}}
 }
 
 \subsection{Arguments}{
@@ -103,9 +100,6 @@ Write (lifecycle: maturing)
 \item{\code{values}}{An \code{\link[arrow:Table-class]{arrow::Table}} or \code{\link[arrow:RecordBatch-class]{arrow::RecordBatch}}
 containing all columns, including any index columns. The
 schema for \code{values} must match the schema for the \code{SOMADataFrame}.}
-
-\item{\code{tsrange}}{An optional two-element Datetime vector for the
-start and end of the timestamp range}
 }
 \if{html}{\out{</div>}}
 }
diff --git a/apis/r/man/SOMANDArrayBase.Rd b/apis/r/man/SOMANDArrayBase.Rd
index 674d0871e0..3e28c56708 100644
--- a/apis/r/man/SOMANDArrayBase.Rd
+++ b/apis/r/man/SOMANDArrayBase.Rd
@@ -61,7 +61,6 @@ experimental)
   type,
   shape,
   platform_config = NULL,
-  timestamps = NULL,
   internal_use_only = NULL
 )}\if{html}{\out{</div>}}
 }
@@ -77,8 +76,6 @@ element in the array.}
 \item{\code{platform_config}}{A \link[tiledbsoma:PlatformConfig]{platform configuration}
 object}
 
-\item{\code{timestamps}}{Optional timestamp start and end range}
-
 \item{\code{internal_use_only}}{Character value to signal this is a 'permitted'
 call, as \code{create()} is considered internal and should not be called
 directly.}
diff --git a/apis/r/man/TileDBArray.Rd b/apis/r/man/TileDBArray.Rd
index cb4cab5b3d..a19cf3823a 100644
--- a/apis/r/man/TileDBArray.Rd
+++ b/apis/r/man/TileDBArray.Rd
@@ -148,15 +148,13 @@ A list of metadata values.
 \subsection{Method \code{set_metadata()}}{
 Add list of metadata to the specified TileDB array. (lifecycle: maturing)
 \subsection{Usage}{
-\if{html}{\out{<div class="r">}}\preformatted{TileDBArray$set_metadata(metadata, timestamps = NULL)}\if{html}{\out{</div>}}
+\if{html}{\out{<div class="r">}}\preformatted{TileDBArray$set_metadata(metadata)}\if{html}{\out{</div>}}
 }
 
 \subsection{Arguments}{
 \if{html}{\out{<div class="arguments">}}
 \describe{
 \item{\code{metadata}}{Named list of metadata to add.}
-
-\item{\code{timestamps}}{Optional two-element vector of timestamp range starts and end}
 }
 \if{html}{\out{</div>}}
 }


Test patch:
diff --git a/apis/r/tests/testthat/test-Timestamps.R b/apis/r/tests/testthat/test-Timestamps.R
index 3e85b4b636..edfc8aa5a9 100644
--- a/apis/r/tests/testthat/test-Timestamps.R
+++ b/apis/r/tests/testthat/test-Timestamps.R
@@ -15,14 +15,16 @@ test_that("SOMADataFrame", {
                                int = 101:105L,
                                str = factor(c('a','b','b','a','b')))
     ts2 <- as.POSIXct(2, tz = "UTC", origin = "1970-01-01")
-    expect_silent(sdf$write(dat2, ts2))
+    expect_no_condition(sdf$reopen(sdf$mode(), tiledb_timestamp = ts2))
+    expect_no_condition(sdf$write(dat2))
 
     ## write part2 at t = 3
     dat3 <- arrow::arrow_table(soma_joinid = bit64::as.integer64(6L:10L),
                                int = 106:110L,
                                str = factor(c('c','b','c','c','b')))
     ts3 <- as.POSIXct(3, tz = "UTC", origin = "1970-01-01")
-    expect_silent(sdf$write(dat3, ts3))
+    expect_no_condition(sdf$reopen(sdf$mode(), tiledb_timestamp = ts3))
+    expect_no_condition(sdf$write(dat3))
     sdf$close()
 
     ## read all
@@ -97,7 +99,7 @@ test_that("SOMANDDenseArray", {
         tiledb_timestamp = ts1
     )
     mat <- create_dense_matrix_with_int_dims(5, 2)
-    expect_silent(dnda$write(mat)) 		# write happens at create time
+    expect_no_condition(dnda$write(mat)) 		# write happens at create time
 
     ## read at t = 3, expect all rows as read is from (0, 3)
     ts2 <- as.POSIXct(3, tz = "UTC", origin = "1970-01-01")