From a39df6a14c3b77ce64bd54b27e904bd55daa09da Mon Sep 17 00:00:00 2001
From: Mark Keller <7525285+keller-mark@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:03:47 -0400
Subject: [PATCH 1/2] Fix zero-element bugs. Add tests. Use file.path in more
 places.

---
 R/array-nested.R             | 39 ++++++++-------
 R/slicing.R                  |  2 +-
 R/zarr-array.R               | 46 +++++++++---------
 tests/testthat/test-compat.R | 94 +++++++++++++++++++++++++-----------
 tests/testthat/test-get.R    |  6 +--
 5 files changed, 116 insertions(+), 71 deletions(-)

diff --git a/R/array-nested.R b/R/array-nested.R
index 4fa92a4..02145e5 100644
--- a/R/array-nested.R
+++ b/R/array-nested.R
@@ -239,24 +239,27 @@ NestedArray <- R6::R6Class("NestedArray",
         message(value)
         stop("Got unexpected type for value in NestedArray$set()")
       }
-      
-      # Cannot figure out how to dynamically set values in an array
-      # of arbitrary dimensions.
-      # Tried: abind::afill <- but it doesn't seem to work with arbitrary dims or do.call
-      if(length(selection_list) == 1) {
-        self$data[selection_list[[1]]] <- value_data
-      } else if(length(selection_list) == 2) {
-        self$data[selection_list[[1]], selection_list[[2]]] <- value_data
-      } else if(length(selection_list) == 3) {
-        self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]]] <- value_data
-      } else if(length(selection_list) == 4) {
-        self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]]] <- value_data
-      } else if(length(selection_list) == 5) {
-        self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]], selection_list[[5]]] <- value_data
-      } else if(length(selection_list) == 6) {
-        self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]], selection_list[[5]], selection_list[[6]]] <- value_data
-      } else {
-        stop("NestedArray$set() can only handle up to 6D arrays at the moment. Please make a feature request if you need to handle more dims.")
+
+      # Only set values if the array is not meant to be empty.
+      if(sum(length(value_data)) > 0 || sum(dim(value_data)) > 0) {
+        # Cannot figure out how to dynamically set values in an array
+        # of arbitrary dimensions.
+        # Tried: abind::afill <- but it doesn't seem to work with arbitrary dims or do.call
+        if(length(selection_list) == 1) {
+          self$data[selection_list[[1]]] <- value_data
+        } else if(length(selection_list) == 2) {
+          self$data[selection_list[[1]], selection_list[[2]]] <- value_data
+        } else if(length(selection_list) == 3) {
+          self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]]] <- value_data
+        } else if(length(selection_list) == 4) {
+          self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]]] <- value_data
+        } else if(length(selection_list) == 5) {
+          self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]], selection_list[[5]]] <- value_data
+        } else if(length(selection_list) == 6) {
+          self$data[selection_list[[1]], selection_list[[2]], selection_list[[3]], selection_list[[4]], selection_list[[5]], selection_list[[6]]] <- value_data
+        } else {
+          stop("NestedArray$set() can only handle up to 6D arrays at the moment. Please make a feature request if you need to handle more dims.")
+        }
       }
     },
     #' @description
diff --git a/R/slicing.R b/R/slicing.R
index 5f114e6..6706aa8 100644
--- a/R/slicing.R
+++ b/R/slicing.R
@@ -119,7 +119,7 @@ slice <- function(start, stop = NA, step = NA, zero_based = FALSE) {
   ))
 }
 
-#' Convenience function for the internal Sliceclass constructor 
+#' Convenience function for the internal Slice class constructor 
 #' with zero-based indexing and exclusive stop index.
 #' @param start The start index.
 #' @param stop The stop index.
diff --git a/R/zarr-array.R b/R/zarr-array.R
index c7570b5..673bd7f 100644
--- a/R/zarr-array.R
+++ b/R/zarr-array.R
@@ -420,30 +420,32 @@ ZarrArray <- R6::R6Class("ZarrArray",
 
       selection_shape <- indexer$shape
       selection_shape_vec <- ensure_integer_vec(indexer$shape)
-      
-      # Check value shape
-      if (length(selection_shape) == 0) {
-        # Setting a single value
-      } else if (is_scalar(value)) {
-        # Setting a scalar value
-      } else if("array" %in% class(value)) {
-        if (!all(ensure_integer_vec(dim(value)) == selection_shape_vec)) {
-          stop("Shape mismatch in source array and set selection: ${dim(value)} and ${selectionShape}")
-        }
-        value <- NestedArray$new(value, shape = selection_shape_vec, dtype=private$dtype, order = private$order)
-      } else if ("NestedArray" %in% class(value)) {
-        if (!all(ensure_integer_vec(value$shape) == selection_shape_vec)) {
-          stop("Shape mismatch in source NestedArray and set selection: ${value.shape} and ${selectionShape}")
+
+      if(sum(as.numeric(selection_shape)) > 0) {
+        # Check value shape
+        if (length(selection_shape) == 0) {
+          # Setting a single value
+        } else if (is_scalar(value)) {
+          # Setting a scalar value
+        } else if("array" %in% class(value)) {
+          if (!all(ensure_integer_vec(dim(value)) == selection_shape_vec)) {
+            stop("Shape mismatch in source array and set selection: ${dim(value)} and ${selectionShape}")
+          }
+          value <- NestedArray$new(value, shape = selection_shape_vec, dtype=private$dtype, order = private$order)
+        } else if ("NestedArray" %in% class(value)) {
+          if (!all(ensure_integer_vec(value$shape) == selection_shape_vec)) {
+            stop("Shape mismatch in source NestedArray and set selection: ${value.shape} and ${selectionShape}")
+          }
+        } else {
+          # // TODO(zarr.js) support TypedArrays, buffers, etc
+          stop("Unknown data type for setting :(")
         }
-      } else {
-        # // TODO(zarr.js) support TypedArrays, buffers, etc
-        stop("Unknown data type for setting :(")
-      }
 
-      # TODO: use queue to handle async iterator
-      for (proj in indexer$iter()) {
-        chunk_value <- private$get_chunk_value(proj, indexer, value, selection_shape)
-        private$chunk_setitem(proj$chunk_coords, proj$chunk_sel, chunk_value)
+        # TODO: use queue to handle async iterator
+        for (proj in indexer$iter()) {
+          chunk_value <- private$get_chunk_value(proj, indexer, value, selection_shape)
+          private$chunk_setitem(proj$chunk_coords, proj$chunk_sel, chunk_value)
+        }
       }
     },
     #' @description
diff --git a/tests/testthat/test-compat.R b/tests/testthat/test-compat.R
index bb08796..d2b0211 100644
--- a/tests/testthat/test-compat.R
+++ b/tests/testthat/test-compat.R
@@ -12,7 +12,7 @@ test_that("Can open Zarr group using convenience function", {
 
 test_that("Can open Zarr group or array using convenience function", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     g <- zarr_open(root)
     a <- zarr_open(root, path="1d.contiguous.lz4.i2")
 
@@ -22,7 +22,7 @@ test_that("Can open Zarr group or array using convenience function", {
 
 test_that("Can open Zarr group and read a 1D 2-byte integer array with LZ4 compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -41,7 +41,7 @@ test_that("Can open Zarr group and read a 1D 2-byte integer array with LZ4 compr
 
 test_that("Can open Zarr group and read a 1D 2-byte integer array with Zstd compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -60,7 +60,7 @@ test_that("Can open Zarr group and read a 1D 2-byte integer array with Zstd comp
 
 test_that("Can open Zarr group and read a 1D 2-byte integer array with Blosc compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -88,7 +88,7 @@ test_that("Can open Zarr group and read a 1D 2-byte integer array with Blosc com
 
 test_that("Can open Zarr group and read a 1D 2-byte integer array with Zlib compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -107,7 +107,7 @@ test_that("Can open Zarr group and read a 1D 2-byte integer array with Zlib comp
 
 test_that("Can open Zarr group and read a 1D 2-byte integer array with no compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -126,7 +126,7 @@ test_that("Can open Zarr group and read a 1D 2-byte integer array with no compre
 
 test_that("Can open Zarr group and read a 1D 4-byte integer array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -143,7 +143,7 @@ test_that("Can open Zarr group and read a 1D 4-byte integer array", {
 
 test_that("Can open Zarr group and read a 1D 1-byte unsigned integer array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -160,7 +160,7 @@ test_that("Can open Zarr group and read a 1D 1-byte unsigned integer array", {
 
 test_that("Can open Zarr group and read a 1D 4-byte float array, little endian", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -177,7 +177,7 @@ test_that("Can open Zarr group and read a 1D 4-byte float array, little endian",
 
 test_that("Can open Zarr group and read a 1D 4-byte float array, big endian", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -194,7 +194,7 @@ test_that("Can open Zarr group and read a 1D 4-byte float array, big endian", {
 
 test_that("Can open Zarr group and read a 1D 8-byte float array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -211,7 +211,7 @@ test_that("Can open Zarr group and read a 1D 8-byte float array", {
 
 test_that("Can open Zarr group and read a 1D 2-byte float array, 2 chunks", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -228,7 +228,7 @@ test_that("Can open Zarr group and read a 1D 2-byte float array, 2 chunks", {
 
 test_that("Can open Zarr group and read a 1D 2-byte float array, 2 chunks, ragged", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -245,7 +245,7 @@ test_that("Can open Zarr group and read a 1D 2-byte float array, 2 chunks, ragge
 
 test_that("Can open Zarr group and read a 2D 2-byte integer array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -263,7 +263,7 @@ test_that("Can open Zarr group and read a 2D 2-byte integer array", {
 
 test_that("Can open Zarr group and read a 2D 2-byte integer array, 2 chunks", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -281,7 +281,7 @@ test_that("Can open Zarr group and read a 2D 2-byte integer array, 2 chunks", {
 
 test_that("Can open Zarr group and read a 2D 2-byte integer array, 2 chunks, ragged", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -301,7 +301,7 @@ test_that("Can open Zarr group and read a 2D 2-byte integer array, 2 chunks, rag
 
 test_that("Can open Zarr group and read a 1D 1-byte boolean array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -318,7 +318,7 @@ test_that("Can open Zarr group and read a 1D 1-byte boolean array", {
 
 test_that("Can open Zarr group and read a 1D S7 string array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -335,7 +335,7 @@ test_that("Can open Zarr group and read a 1D S7 string array", {
 
 test_that("Can open Zarr group and read a 1D U7 string array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -352,7 +352,7 @@ test_that("Can open Zarr group and read a 1D U7 string array", {
 
 test_that("Can open Zarr group and read a 1D U13 little endian string array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -369,7 +369,7 @@ test_that("Can open Zarr group and read a 1D U13 little endian string array", {
 
 test_that("Can open Zarr group and read a 1D U13 big endian string array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -386,7 +386,7 @@ test_that("Can open Zarr group and read a 1D U13 big endian string array", {
 
 test_that("Can open Zarr group and read a 2D U7 string array", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -403,7 +403,7 @@ test_that("Can open Zarr group and read a 2D U7 string array", {
 
 test_that("Can open Zarr group and read a 1D VLen-UTF8 string array with no compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -420,7 +420,7 @@ test_that("Can open Zarr group and read a 1D VLen-UTF8 string array with no comp
 
 test_that("Can open Zarr group and read a 1D VLen-UTF8 string array with Blosc compression", {
 
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -436,7 +436,7 @@ test_that("Can open Zarr group and read a 1D VLen-UTF8 string array with Blosc c
 })
 
 test_that("Can open Zarr group and read a 2D VLen-UTF8 string array with no compression", {
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -452,7 +452,7 @@ test_that("Can open Zarr group and read a 2D VLen-UTF8 string array with no comp
 })
 
 test_that("Can open Zarr group and read a 2D VLen-UTF8 string array with Blosc compression", {
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     g <- ZarrGroup$new(store)
@@ -468,9 +468,49 @@ test_that("Can open Zarr group and read a 2D VLen-UTF8 string array with Blosc c
 })
 
 test_that("DirectoryStore can listdir", {
-    root <- pizzarr_sample("fixtures/v2/data.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "data.zarr"))
     
     store <- DirectoryStore$new(root)
     
     expect_equal(store$listdir("1d.chunked.i2"), c(".zarray", "0", "1"))
 })
+
+test_that("Can create 0-element 1D array", {
+
+    store <- MemoryStore$new()
+    value <- character(0)
+    dims <- c(0)
+    name <- "my-0-element-array-1d"
+
+    object_codec <- VLenUtf8Codec$new()
+    data <- array(data = value, dim = dims)
+    a <- zarr_create_array(data, store = store, path = name, dtype = "|O", object_codec = object_codec, shape = dims)
+
+    expect_equal(a$get_shape(), c(0))
+
+    sel <- a$get_item("...")
+    
+    expect_equal(is.character(sel$data), TRUE)
+    expect_equal(length(sel$data), 0)
+    expect_equal(dim(sel$data), c(0))
+})
+
+test_that("Can create 0-element 2D array", {
+
+    store <- MemoryStore$new()
+    value <- character(0)
+    dims <- c(0, 0)
+    name <- "my-0-element-array-2d"
+
+    object_codec <- VLenUtf8Codec$new()
+    data <- array(data = value, dim = dims)
+    a <- zarr_create_array(data, store = store, path = name, dtype = "|O", object_codec = object_codec, shape = dims)
+
+    expect_equal(a$get_shape(), c(0, 0))
+
+    sel <- a$get_item("...")
+    
+    expect_equal(is.character(sel$data), TRUE)
+    expect_equal(length(sel$data), 0)
+    expect_equal(dim(sel$data), c(0, 0))
+})
\ No newline at end of file
diff --git a/tests/testthat/test-get.R b/tests/testthat/test-get.R
index 85dc935..d1d6097 100644
--- a/tests/testthat/test-get.R
+++ b/tests/testthat/test-get.R
@@ -16,7 +16,7 @@ test_that("get_basic_selection_zd", {
 })
 
 test_that("get_basic_selection_zd with anndataR IntScalar fixture", {
-    root <- pizzarr_sample("fixtures/v2/example.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "example.zarr"))
     
     store <- DirectoryStore$new(root)
     z <- zarr_open_array(store, path = "uns/IntScalar")
@@ -30,7 +30,7 @@ test_that("get_basic_selection_zd with anndataR IntScalar fixture", {
 })
 
 test_that("get_basic_selection_zd with anndataR StringScalar fixture", {
-    root <- pizzarr_sample("fixtures/v2/example.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "example.zarr"))
     
     store <- DirectoryStore$new(root)
     z <- zarr_open_array(store, path = "uns/StringScalar")
@@ -256,7 +256,7 @@ test_that("get_basic_selection_3d(one-based) - can get_item for three-dimensiona
 })
 
 test_that("Can read 2D string array", {
-    root <- pizzarr_sample("fixtures/v2/example.zarr")
+    root <- pizzarr_sample(file.path("fixtures", "v2", "example.zarr"))
     store <- DirectoryStore$new(root)
     z <- zarr_open_array(store, path = "uns/String2D")
     nested_arr <- z$get_item("...")

From 94742686e659cd6a34065b9c476c1d15a694cbff Mon Sep 17 00:00:00 2001
From: Mark Keller <7525285+keller-mark@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:32:03 -0400
Subject: [PATCH 2/2] Test for array contents in step size 2 and 3 test

---
 tests/testthat/test-slice-indices.R | 32 ++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-slice-indices.R b/tests/testthat/test-slice-indices.R
index 3988df1..3683142 100644
--- a/tests/testthat/test-slice-indices.R
+++ b/tests/testthat/test-slice-indices.R
@@ -41,9 +41,35 @@ test_that("slice gets converted to zb_slice", {
 test_that("step size greater than 1", {
   g <- zarr_volcano()
   
-  s <- slice(1, 21, 2)
+  s1 <- slice(1, 11, 1)
+  s2 <- slice(1, 11, 2)
+  s3 <- slice(1, 11, 3)
   
-  d <- g$get_item("volcano")$get_item(list(s, s))$data
+  d1 <- g$get_item("volcano")$get_item(list(s1, s1))$data
+  d2 <- g$get_item("volcano")$get_item(list(s2, s2))$data
+  d3 <- g$get_item("volcano")$get_item(list(s3, s3))$data
   
-  expect_equal(dim(d), c(11, 11))
+  expect_equal(dim(d1), c(11, 11))
+  expect_equal(dim(d2), c(6, 6))
+  expect_equal(dim(d3), c(4, 4))
+
+  # ground truth for the volcano section with step size 2
+  a2 <- t(array(data=c(
+    100, 101, 101, 101, 100, 101,
+    102, 103, 103, 103, 102, 103,
+    104, 105, 105, 105, 104, 104,
+    105, 106, 107, 107, 106, 105,
+    107, 108, 109, 109, 108, 108,
+    109, 110, 111, 111, 110, 112
+  ), dim=c(6, 6)))
+  expect_equal(d2, a2)
+
+  # ground truth for the volcano section with step size 3
+  a3 <- t(array(data=c(
+    100, 101, 101, 100,
+    103, 104, 104, 103,
+    105, 107, 107, 105,
+    108, 110, 110, 108
+  ), dim=c(4, 4)))
+  expect_equal(d3, a3)
 })
\ No newline at end of file