code-review feedback

apis/r/R/SOMADataFrame.R

@@ -396,23 +396,14 @@ SOMADataFrame <- R6::R6Class(
       has_current_domain(self$uri, private$.soma_context)
     },
 
-    #' @description Increases the shape of the array as specfied. Raises an error
-    #' if the new shape is less than the current shape in any dimension. Raises
-    #' an error if the new shape exceeds maxshape in any dimension. Raises an
-    #' error if the array doesn't already have a shape: in that case please call
-    #' tiledbsoma_upgrade_shape.
-    #' @param new_shape A vector of integerish, of the same length as the array's `ndim`.
-    #' @return No return value
-
-
-    # @description Increases the shape of the dataframe on the ``soma_joinid``
-    # index column, if it indeed is an index column, leaving all other index
-    # columns as-is. If the ``soma_joinid`` is not an index column, no change is
-    # made.  This is a special case of ``upgrade_domain`` (WIP for 1.15), but
-    # simpler to keystroke, and handles the most common case for dataframe
-    # domain expansion.  Raises an error if the dataframe doesn't already have a
-    # domain: in that case please call ``tiledbsoma_upgrade_domain`` (WIP for
-    # 1.15).
+    #' @description Increases the shape of the dataframe on the ``soma_joinid``
+    #' index column, if it indeed is an index column, leaving all other index
+    #' columns as-is. If the ``soma_joinid`` is not an index column, no change is
+    #' made.  This is a special case of ``upgrade_domain`` (WIP for 1.15), but
+    #' simpler to keystroke, and handles the most common case for dataframe
+    #' domain expansion.  Raises an error if the dataframe doesn't already have a
+    #' domain: in that case please call ``tiledbsoma_upgrade_domain`` (WIP for
+    #' 1.15).
     #' @param new_shape An integer, greater than or equal to 1 + the
     #' `soma_joinid` domain slot.
     #' @return No return value
@@ -422,7 +413,7 @@ SOMADataFrame <- R6::R6Class(
         (bit64::is.integer64(new_shape) && length(new_shape) == 1)
       )
       # Checking slotwise new shape >= old shape, and <= max_shape, is already done in libtiledbsoma
-      resize_soma_joinid(self$uri, new_shape, private$.soma_context)
+      invisible(resize_soma_joinid(self$uri, new_shape, private$.soma_context))
     }
 
   ),

apis/r/tests/testthat/test-shape.R

@@ -232,6 +232,7 @@ test_that("SOMADataFrame shape", {
         }
         sdf$close()
 
+        # Make sure the failed resize really didn't change the shape
         if (has_soma_joinid_dim) {
           sdf <- SOMADataFrameOpen(uri, "READ")
           expect_equal(sdf$domain()[["soma_joinid"]], sjid_dfc)
@@ -253,7 +254,6 @@ test_that("SOMADataFrame shape", {
           schema = asch)
 
         sdf <- SOMADataFrameOpen(uri, "WRITE")
-        # Current status: debug
         if (has_soma_joinid_dim) {
           expect_error(sdf$write(tbl1))
         } else {
@@ -262,17 +262,17 @@ test_that("SOMADataFrame shape", {
         sdf$close()
 
         # Test resize
-
         sdf <- SOMADataFrameOpen(uri, "WRITE")
         sdf$resize_soma_joinid(new_shape)
         sdf$close();
 
         # Test writes out of old bounds, within new bounds, after resize
         sdf <- SOMADataFrameOpen(uri, "WRITE")
-
         expect_no_condition(sdf$write(tbl1))
         sdf$close();
 
+        # To do: test readback
+
         rm(tbl1)
       }