From 4aa8cae71a1bdf2c066e167180e0e8e7d36b0406 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Thu, 20 Jun 2024 21:30:06 +0000
Subject: [PATCH 01/46] Implemented concept of "open as byte array" for any
 dtype

---
 tensorstore/driver/zarr/dtype.cc     | 19 ++++++++++++++++---
 tensorstore/driver/zarr/dtype.h      |  5 ++++-
 tensorstore/driver/zarr/metadata.cc  | 25 ++++++++++++++++++++++++-
 tensorstore/driver/zarr/metadata.h   |  2 +-
 tensorstore/driver/zarr/spec.cc      |  2 +-
 tensorstore/driver/zarr/spec_test.cc |  3 +++
 6 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 47415f7a9..b87f8f1f1 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -214,7 +214,7 @@ namespace {
 /// \param value The zarr metadata "dtype" JSON specification.
 /// \param out[out] Must be non-null.  Filled with the parsed dtype on success.
 /// \error `absl::StatusCode::kInvalidArgument' if `value` is invalid.
-Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value) {
+Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDTypeOptions& options) {
   ZarrDType out;
   if (value.is_string()) {
     // Single field.
@@ -279,6 +279,19 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value) {
             });
       });
   if (!parse_result.ok()) return parse_result;
+ 
+  if (options.treat_struct_as_byte_array) {
+    // Convert struct dtype to a single byte array dtype.
+    out.has_fields = false;
+    ZarrDType::Field byte_array_field;
+    byte_array_field.name = "";
+    byte_array_field.dtype = dtype_v<std::byte>;
+    byte_array_field.endian = endian::native;
+    byte_array_field.encoded_dtype = "V";
+    byte_array_field.flexible_shape = {out.bytes_per_outer_element};
+    out.fields = {byte_array_field};
+  }
+ 
   return out;
 }
 
@@ -320,8 +333,8 @@ absl::Status ValidateDType(ZarrDType& dtype) {
   return absl::OkStatus();
 }
 
-Result<ZarrDType> ParseDType(const nlohmann::json& value) {
-  TENSORSTORE_ASSIGN_OR_RETURN(ZarrDType dtype, ParseDTypeNoDerived(value));
+Result<ZarrDType> ParseDType(const nlohmann::json& value, const ParseDTypeOptions& options) {
+  TENSORSTORE_ASSIGN_OR_RETURN(ZarrDType dtype, ParseDTypeNoDerived(value, options));
   TENSORSTORE_RETURN_IF_ERROR(ValidateDType(dtype));
   return dtype;
 }
diff --git a/tensorstore/driver/zarr/dtype.h b/tensorstore/driver/zarr/dtype.h
index be858d671..1512b354e 100644
--- a/tensorstore/driver/zarr/dtype.h
+++ b/tensorstore/driver/zarr/dtype.h
@@ -121,10 +121,13 @@ struct ZarrDType {
                       const ZarrDType& dtype);
 };
 
+struct ParseDTypeOptions {
+  bool treat_struct_as_byte_array = false;
+};
 /// Parses a zarr metadata "dtype" JSON specification.
 ///
 /// \error `absl::StatusCode::kInvalidArgument` if `value` is not valid.
-Result<ZarrDType> ParseDType(const ::nlohmann::json& value);
+Result<ZarrDType> ParseDType(const ::nlohmann::json& value, const ParseDTypeOptions& options = {});
 
 /// Validates `dtype and computes derived values.
 ///
diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index e69c6bcf0..466991ce3 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -445,9 +445,32 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(ZarrPartialMetadata,
 // only support raw decoder.
 
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
-    const ZarrMetadata& metadata, absl::Cord buffer) {
+    const ZarrMetadata& metadata, absl::Cord buffer, bool treat_struct_as_byte_array) {
   const size_t num_fields = metadata.dtype.fields.size();
   absl::InlinedVector<SharedArray<const void>, 1> field_arrays(num_fields);
+  if (treat_struct_as_byte_array) {
+    // Treat the entire chunk as a single byte array.
+    SharedArray<const void> byte_array;
+    if (metadata.compressor) {
+      std::unique_ptr<riegeli::Reader> reader =
+          std::make_unique<riegeli::CordReader<absl::Cord>>(std::move(buffer));
+      reader = metadata.compressor->GetReader(
+          std::move(reader), metadata.dtype.bytes_per_outer_element);
+      TENSORSTORE_RETURN_IF_ERROR(riegeli::ReadAll(std::move(reader), buffer));
+    }
+    byte_array = AllocateArray(
+        {metadata.chunk_layout.bytes_per_chunk}, ContiguousLayoutOrder::c,
+        default_init, dtype_v<std::byte>);
+    std::string buffer_str(buffer.Flatten()); // TODO: Does this really need to be a string? What's going on here?
+    if (byte_array.num_elements() >= buffer_str.size()) {
+        std::memcpy(const_cast<void*>(byte_array.data()), buffer_str.data(), buffer_str.size());
+    } else {
+      return absl::InvalidArgumentError("byte_array is not large enough to hold buffer_str");
+    }
+    field_arrays[0] = std::move(byte_array);
+    return field_arrays;
+  }
+
   if (num_fields == 1) {
     // Optimized code path, decompress directly into output array.
     const auto& dtype_field = metadata.dtype.fields[0];
diff --git a/tensorstore/driver/zarr/metadata.h b/tensorstore/driver/zarr/metadata.h
index 6c4588554..a2712e202 100644
--- a/tensorstore/driver/zarr/metadata.h
+++ b/tensorstore/driver/zarr/metadata.h
@@ -241,7 +241,7 @@ Result<absl::Cord> EncodeChunk(
 /// \error `absl::StatusCode::kInvalidArgument` if `buffer` is not a valid
 ///     encoded zarr chunk according to `metadata`.
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
-    const ZarrMetadata& metadata, absl::Cord buffer);
+    const ZarrMetadata& metadata, absl::Cord buffer, bool treat_struct_as_byte_array = false);
 
 /// Returns `true` if `a` and `b` are compatible, meaning stored data created
 /// with `a` can be read using `b`.
diff --git a/tensorstore/driver/zarr/spec.cc b/tensorstore/driver/zarr/spec.cc
index 8fac3a4e2..4135e8d51 100644
--- a/tensorstore/driver/zarr/spec.cc
+++ b/tensorstore/driver/zarr/spec.cc
@@ -525,7 +525,7 @@ std::string GetFieldNames(const ZarrDType& dtype) {
 Result<size_t> GetFieldIndex(const ZarrDType& dtype,
                              const SelectedField& selected_field) {
   if (selected_field.empty()) {
-    if (dtype.fields.size() != 1) {
+    if (dtype.fields.size() != 1 && !dtype.fields.back().name.empty()) {
       return absl::FailedPreconditionError(tensorstore::StrCat(
           "Must specify a \"field\" that is one of: ", GetFieldNames(dtype)));
     }
diff --git a/tensorstore/driver/zarr/spec_test.cc b/tensorstore/driver/zarr/spec_test.cc
index 93004dcd2..a04651c4e 100644
--- a/tensorstore/driver/zarr/spec_test.cc
+++ b/tensorstore/driver/zarr/spec_test.cc
@@ -186,6 +186,9 @@ TEST(GetFieldIndexTest, Null) {
       MatchesStatus(
           absl::StatusCode::kFailedPrecondition,
           "Must specify a \"field\" that is one of: \\[\"x\",\"y\"\\]"));
+
+  auto res = GetFieldIndex(ParseDType(::nlohmann::json::array_t{{"x", "<i4"}, {"y", "<u2"}}, {true}).value(), "");
+  ASSERT_TRUE(res.status().ok()) << res.status();
 }
 
 TEST(GetFieldIndexTest, String) {

From ac6ba5c9d3dce30c0060a7cde709722f07fc0e37 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 13:47:07 +0000
Subject: [PATCH 02/46] Added open option for byte array

---
 tensorstore/open_mode.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tensorstore/open_mode.h b/tensorstore/open_mode.h
index 9b48e14c1..61b8e668d 100644
--- a/tensorstore/open_mode.h
+++ b/tensorstore/open_mode.h
@@ -67,6 +67,10 @@ enum class OpenMode {
   /// This option must be specified in conjunction with `open` and must not be
   /// specified in conjunction with `delete_existing`.
   assume_cached_metadata = 16,
+
+  /// Open the TensorStore as a byte array instead of converting everything to
+  /// it's native dtype.
+  treat_struct_as_byte_array = 32,
 };
 
 /// Returns the intersection of two open modes.

From 6b43911ff9b7b624f50bb2de3ade8d90e088b51e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 15:58:20 +0000
Subject: [PATCH 03/46] Removed open option for byte array

---
 tensorstore/open_mode.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tensorstore/open_mode.h b/tensorstore/open_mode.h
index 61b8e668d..9b48e14c1 100644
--- a/tensorstore/open_mode.h
+++ b/tensorstore/open_mode.h
@@ -67,10 +67,6 @@ enum class OpenMode {
   /// This option must be specified in conjunction with `open` and must not be
   /// specified in conjunction with `delete_existing`.
   assume_cached_metadata = 16,
-
-  /// Open the TensorStore as a byte array instead of converting everything to
-  /// it's native dtype.
-  treat_struct_as_byte_array = 32,
 };
 
 /// Returns the intersection of two open modes.

From c4beb115fe96c5e3256fd35756ad234b4bceb254 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 15:58:53 +0000
Subject: [PATCH 04/46] Allow modification of ParseDTypeOptions

---
 tensorstore/driver/zarr/dtype.cc | 3 ++-
 tensorstore/driver/zarr/dtype.h  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index b87f8f1f1..695dfdf87 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -214,9 +214,10 @@ namespace {
 /// \param value The zarr metadata "dtype" JSON specification.
 /// \param out[out] Must be non-null.  Filled with the parsed dtype on success.
 /// \error `absl::StatusCode::kInvalidArgument' if `value` is invalid.
-Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDTypeOptions& options) {
+Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOptions& options) {
   ZarrDType out;
   if (value.is_string()) {
+    options.treat_struct_as_byte_array = false; // We don't want to treat a single value as struct.
     // Single field.
     out.has_fields = false;
     out.fields.resize(1);
diff --git a/tensorstore/driver/zarr/dtype.h b/tensorstore/driver/zarr/dtype.h
index 1512b354e..2b16c5b4d 100644
--- a/tensorstore/driver/zarr/dtype.h
+++ b/tensorstore/driver/zarr/dtype.h
@@ -127,7 +127,7 @@ struct ParseDTypeOptions {
 /// Parses a zarr metadata "dtype" JSON specification.
 ///
 /// \error `absl::StatusCode::kInvalidArgument` if `value` is not valid.
-Result<ZarrDType> ParseDType(const ::nlohmann::json& value, const ParseDTypeOptions& options = {});
+Result<ZarrDType> ParseDType(const ::nlohmann::json& value, ParseDTypeOptions& options = {});
 
 /// Validates `dtype and computes derived values.
 ///

From 43cc7c8d0815e8850be485eedc090b3e875e0281 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 15:59:11 +0000
Subject: [PATCH 05/46] Update default behavior

---
 tensorstore/driver/zarr/dtype.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/dtype.h b/tensorstore/driver/zarr/dtype.h
index 2b16c5b4d..2c76a8d61 100644
--- a/tensorstore/driver/zarr/dtype.h
+++ b/tensorstore/driver/zarr/dtype.h
@@ -122,7 +122,7 @@ struct ZarrDType {
 };
 
 struct ParseDTypeOptions {
-  bool treat_struct_as_byte_array = false;
+  bool treat_struct_as_byte_array = true;
 };
 /// Parses a zarr metadata "dtype" JSON specification.
 ///

From a76363aea473e023a9e81b9ca6bf4ef056e4d29f Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 15:59:35 +0000
Subject: [PATCH 06/46] Emplace instead of replace

---
 tensorstore/driver/zarr/dtype.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 695dfdf87..30c973659 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -281,6 +281,7 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOpt
       });
   if (!parse_result.ok()) return parse_result;
  
+  // TODO: How can I check that there wasn't a field provided and that it is structarray?
   if (options.treat_struct_as_byte_array) {
     // Convert struct dtype to a single byte array dtype.
     out.has_fields = false;
@@ -290,7 +291,8 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOpt
     byte_array_field.endian = endian::native;
     byte_array_field.encoded_dtype = "V";
     byte_array_field.flexible_shape = {out.bytes_per_outer_element};
-    out.fields = {byte_array_field};
+    out.fields.emplace_back({byte_array_field});
+    // out.fields = {byte_array_field};
   }
  
   return out;
@@ -334,7 +336,7 @@ absl::Status ValidateDType(ZarrDType& dtype) {
   return absl::OkStatus();
 }
 
-Result<ZarrDType> ParseDType(const nlohmann::json& value, const ParseDTypeOptions& options) {
+Result<ZarrDType> ParseDType(const nlohmann::json& value, ParseDTypeOptions& options) {
   TENSORSTORE_ASSIGN_OR_RETURN(ZarrDType dtype, ParseDTypeNoDerived(value, options));
   TENSORSTORE_RETURN_IF_ERROR(ValidateDType(dtype));
   return dtype;

From 63424177632799b402dbda6d8acff9b763345267 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 21 Jun 2024 16:00:04 +0000
Subject: [PATCH 07/46] Update field selection to use emplaced field

---
 tensorstore/driver/zarr/spec.cc | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tensorstore/driver/zarr/spec.cc b/tensorstore/driver/zarr/spec.cc
index 4135e8d51..a7399f056 100644
--- a/tensorstore/driver/zarr/spec.cc
+++ b/tensorstore/driver/zarr/spec.cc
@@ -525,11 +525,14 @@ std::string GetFieldNames(const ZarrDType& dtype) {
 Result<size_t> GetFieldIndex(const ZarrDType& dtype,
                              const SelectedField& selected_field) {
   if (selected_field.empty()) {
-    if (dtype.fields.size() != 1 && !dtype.fields.back().name.empty()) {
-      return absl::FailedPreconditionError(tensorstore::StrCat(
-          "Must specify a \"field\" that is one of: ", GetFieldNames(dtype)));
+    std::size_t size = dtype.fields.size();
+    if (size == 1) {
+      return 0;
+    } else if (dtype.fields.back().name.empty()) {
+      return size-1;
     }
-    return 0;
+    return absl::FailedPreconditionError(tensorstore::StrCat(
+        "Must specify a \"field\" that is one of: ", GetFieldNames(dtype)));
   }
   if (!dtype.has_fields) {
     return absl::FailedPreconditionError(

From f684f17163ef6dfd2d53804049b9356378ef3603 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:53:49 +0000
Subject: [PATCH 08/46] Stopped synthetic dimension from getting seralized

---
 tensorstore/driver/zarr/driver.cc | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index a3a3f9d42..438c25e3a 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -121,8 +121,12 @@ Result<MetadataCache::MetadataPtr> MetadataCache::DecodeMetadata(
 
 Result<absl::Cord> MetadataCache::EncodeMetadata(std::string_view entry_key,
                                                  const void* metadata) {
-  return absl::Cord(
-      ::nlohmann::json(*static_cast<const ZarrMetadata*>(metadata)).dump());
+  auto meta = ::nlohmann::json(*static_cast<const ZarrMetadata*>(metadata));
+  if (meta["dtype"].is_array() && meta["dtype"].back()[0] == "") {
+    meta["dtype"].erase(meta["dtype"].size() - 1);
+  }
+
+  return absl::Cord(meta.dump());
 }
 
 absl::Status ZarrDriverSpec::ApplyOptions(SpecOptions&& options) {

From 93e92ce2dec821f00b58225779734e1fd919d78f Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:54:31 +0000
Subject: [PATCH 09/46] Pushback synthetic field rather than make it the only
 field

---
 tensorstore/driver/zarr/dtype.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 30c973659..1dd19bee3 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -291,10 +291,8 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOpt
     byte_array_field.endian = endian::native;
     byte_array_field.encoded_dtype = "V";
     byte_array_field.flexible_shape = {out.bytes_per_outer_element};
-    out.fields.emplace_back({byte_array_field});
-    // out.fields = {byte_array_field};
+    out.fields.push_back({byte_array_field});
   }
- 
   return out;
 }
 

From 874d581f1db83d47fd59e0bef698c876a0ec848e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:55:04 +0000
Subject: [PATCH 10/46] Skip the synthetic field as part of the number of bytes

---
 tensorstore/driver/zarr/dtype.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 1dd19bee3..cb8375667 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -301,6 +301,9 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOpt
 absl::Status ValidateDType(ZarrDType& dtype) {
   dtype.bytes_per_outer_element = 0;
   for (size_t field_i = 0; field_i < dtype.fields.size(); ++field_i) {
+    if (dtype.fields[field_i].name.empty()) {
+      continue;
+    }
     auto& field = dtype.fields[field_i];
     if (std::any_of(
             dtype.fields.begin(), dtype.fields.begin() + field_i,

From c7408d478804469d6171c23e2757b3a6a95288ba Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:55:29 +0000
Subject: [PATCH 11/46] Fix const qualifier

---
 tensorstore/driver/zarr/dtype.cc | 4 ++--
 tensorstore/driver/zarr/dtype.h  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index cb8375667..ca0696f2c 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -214,7 +214,7 @@ namespace {
 /// \param value The zarr metadata "dtype" JSON specification.
 /// \param out[out] Must be non-null.  Filled with the parsed dtype on success.
 /// \error `absl::StatusCode::kInvalidArgument' if `value` is invalid.
-Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, ParseDTypeOptions& options) {
+Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDTypeOptions& options) {
   ZarrDType out;
   if (value.is_string()) {
     options.treat_struct_as_byte_array = false; // We don't want to treat a single value as struct.
@@ -337,7 +337,7 @@ absl::Status ValidateDType(ZarrDType& dtype) {
   return absl::OkStatus();
 }
 
-Result<ZarrDType> ParseDType(const nlohmann::json& value, ParseDTypeOptions& options) {
+Result<ZarrDType> ParseDType(const nlohmann::json& value, const ParseDTypeOptions& options) {
   TENSORSTORE_ASSIGN_OR_RETURN(ZarrDType dtype, ParseDTypeNoDerived(value, options));
   TENSORSTORE_RETURN_IF_ERROR(ValidateDType(dtype));
   return dtype;
diff --git a/tensorstore/driver/zarr/dtype.h b/tensorstore/driver/zarr/dtype.h
index 2c76a8d61..c3882d1bb 100644
--- a/tensorstore/driver/zarr/dtype.h
+++ b/tensorstore/driver/zarr/dtype.h
@@ -127,7 +127,7 @@ struct ParseDTypeOptions {
 /// Parses a zarr metadata "dtype" JSON specification.
 ///
 /// \error `absl::StatusCode::kInvalidArgument` if `value` is not valid.
-Result<ZarrDType> ParseDType(const ::nlohmann::json& value, ParseDTypeOptions& options = {});
+Result<ZarrDType> ParseDType(const ::nlohmann::json& value, const ParseDTypeOptions& options = {});
 
 /// Validates `dtype and computes derived values.
 ///

From 83acfa6ffa6339d3588b5a005d8632296c7ef46c Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:55:56 +0000
Subject: [PATCH 12/46] Properly modify const object

---
 tensorstore/driver/zarr/dtype.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index ca0696f2c..9ec242672 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -217,7 +217,7 @@ namespace {
 Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDTypeOptions& options) {
   ZarrDType out;
   if (value.is_string()) {
-    options.treat_struct_as_byte_array = false; // We don't want to treat a single value as struct.
+    const_cast<ParseDTypeOptions&>(options).treat_struct_as_byte_array = false;
     // Single field.
     out.has_fields = false;
     out.fields.resize(1);

From 7ac53a4fa4e056964e8ce552c258e5dd8b424c1d Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 14:57:50 +0000
Subject: [PATCH 13/46] Properly modify has_fields logic

---
 tensorstore/driver/zarr/metadata.cc | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 466991ce3..61df05306 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -270,10 +270,10 @@ Result<std::vector<SharedArray<const void>>> ParseFillValue(
 ::nlohmann::json EncodeFillValue(
     const ZarrDType& dtype, span<const SharedArray<const void>> fill_values) {
   assert(dtype.fields.size() == static_cast<size_t>(fill_values.size()));
-  if (!dtype.has_fields) {
-    assert(dtype.fields.size() == 1);
-    const auto& field = dtype.fields[0];
-    const auto& fill_value = fill_values[0];
+  if (!dtype.has_fields && dtype.fields.back().name != "") {
+    std::size_t idx = 0;
+    const auto& field = dtype.fields[idx];
+    const auto& fill_value = fill_values[idx];
     if (!fill_value.valid()) return nullptr;
     char type_indicator = GetTypeIndicator(field.encoded_dtype);
     switch (type_indicator) {
@@ -443,12 +443,15 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(ZarrPartialMetadata,
 
 // Two decoding strategies:  raw decoder and custom decoder.  Initially we will
 // only support raw decoder.
-
+// TODO: Remove the bool arg.
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
     const ZarrMetadata& metadata, absl::Cord buffer, bool treat_struct_as_byte_array) {
   const size_t num_fields = metadata.dtype.fields.size();
   absl::InlinedVector<SharedArray<const void>, 1> field_arrays(num_fields);
-  if (treat_struct_as_byte_array) {
+
+  std::string back_field = metadata.dtype.fields.back().name;
+  if (back_field == "") {
+    const_cast<ZarrMetadata&>(metadata).dtype.has_fields = false;
     // Treat the entire chunk as a single byte array.
     SharedArray<const void> byte_array;
     if (metadata.compressor) {

From 2c0bdc33950a4cad79b0bc16e9f05df7cdfd3e04 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 15:30:50 +0000
Subject: [PATCH 14/46] Pulled check outside loop and fixed logic error

---
 tensorstore/driver/zarr/dtype.cc | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 9ec242672..0a54b39f9 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -300,10 +300,11 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
 
 absl::Status ValidateDType(ZarrDType& dtype) {
   dtype.bytes_per_outer_element = 0;
-  for (size_t field_i = 0; field_i < dtype.fields.size(); ++field_i) {
-    if (dtype.fields[field_i].name.empty()) {
-      continue;
-    }
+  size_t num_fields = dtype.fields.size();
+  if (dtype.fields.back().name.empty() && dtype.fields.size() > 1) {
+    --num_fields;
+  }
+  for (size_t field_i = 0; field_i < num_fields; ++field_i) {
     auto& field = dtype.fields[field_i];
     if (std::any_of(
             dtype.fields.begin(), dtype.fields.begin() + field_i,

From 442daa056ed60255277c2860821415f5c8c42eb5 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 16:18:49 +0000
Subject: [PATCH 15/46] Explicit initilization of values. Ensured has_fields
 for structured data without field

---
 tensorstore/driver/zarr/dtype.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 0a54b39f9..1d888bab2 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -284,13 +284,16 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
   // TODO: How can I check that there wasn't a field provided and that it is structarray?
   if (options.treat_struct_as_byte_array) {
     // Convert struct dtype to a single byte array dtype.
-    out.has_fields = false;
+    // out.has_fields = false;
     ZarrDType::Field byte_array_field;
     byte_array_field.name = "";
     byte_array_field.dtype = dtype_v<std::byte>;
     byte_array_field.endian = endian::native;
     byte_array_field.encoded_dtype = "V";
     byte_array_field.flexible_shape = {out.bytes_per_outer_element};
+    byte_array_field.num_inner_elements = 0; // I don't think I need to set this explicitly
+    byte_array_field.byte_offset = 0; // I don't think I need to set this explicitly
+    byte_array_field.num_bytes = 0; // This will get set properly elsewhere, but let's init it to 0 anyway
     out.fields.push_back({byte_array_field});
   }
   return out;

From 92145fc52f96f0dbaa9230f0388cbb9f25f4846f Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 16:19:47 +0000
Subject: [PATCH 16/46] Implemented proper field_shape for synthetic field

---
 tensorstore/driver/zarr/dtype.cc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 1d888bab2..2ae9d9b42 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -304,8 +304,11 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
 absl::Status ValidateDType(ZarrDType& dtype) {
   dtype.bytes_per_outer_element = 0;
   size_t num_fields = dtype.fields.size();
+  // TODO: Implement better logic and name
+  bool flag = false;
   if (dtype.fields.back().name.empty() && dtype.fields.size() > 1) {
     --num_fields;
+    flag = true;
   }
   for (size_t field_i = 0; field_i < num_fields; ++field_i) {
     auto& field = dtype.fields[field_i];
@@ -338,6 +341,9 @@ absl::Status ValidateDType(ZarrDType& dtype) {
           "Total number of bytes per outer array element is too large");
     }
   }
+  if (flag) {
+    dtype.fields[num_fields].field_shape = {dtype.bytes_per_outer_element};
+  }
   return absl::OkStatus();
 }
 

From 77ce1cabdc19ce9c1e47948d32a56e01564b6ac9 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 16:20:05 +0000
Subject: [PATCH 17/46] Added valid case for an empty named field

---
 tensorstore/driver/zarr/dtype.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 2ae9d9b42..bdb127efb 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -248,6 +248,10 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
               switch (i) {
                 case 0:
                   if (internal_json::JsonRequireValueAs(v, &field.name).ok()) {
+                    // This SHOULD only be the case if a field was not provided
+                    if (field_i == out.fields.size() - 1 && field.name.empty()) {
+                      return absl::OkStatus();
+                    }
                     if (!field.name.empty()) return absl::OkStatus();
                   }
                   return absl::InvalidArgumentError(tensorstore::StrCat(

From 1113ec7e5210eaad4b3d6fb261c78bad053f406f Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 16:20:49 +0000
Subject: [PATCH 18/46] Disabled setting has_fields to false

---
 tensorstore/driver/zarr/metadata.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 61df05306..362e87e78 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -451,7 +451,7 @@ Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
 
   std::string back_field = metadata.dtype.fields.back().name;
   if (back_field == "") {
-    const_cast<ZarrMetadata&>(metadata).dtype.has_fields = false;
+    // const_cast<ZarrMetadata&>(metadata).dtype.has_fields = false;
     // Treat the entire chunk as a single byte array.
     SharedArray<const void> byte_array;
     if (metadata.compressor) {

From 9d9a555893103b740f6347f85337050608a6d44c Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 24 Jun 2024 17:33:22 +0000
Subject: [PATCH 19/46] Fix encoding

---
 tensorstore/driver/zarr/dtype.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index bdb127efb..25a7fc951 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -293,7 +293,7 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
     byte_array_field.name = "";
     byte_array_field.dtype = dtype_v<std::byte>;
     byte_array_field.endian = endian::native;
-    byte_array_field.encoded_dtype = "V";
+    byte_array_field.encoded_dtype = "|V";
     byte_array_field.flexible_shape = {out.bytes_per_outer_element};
     byte_array_field.num_inner_elements = 0; // I don't think I need to set this explicitly
     byte_array_field.byte_offset = 0; // I don't think I need to set this explicitly
@@ -347,6 +347,7 @@ absl::Status ValidateDType(ZarrDType& dtype) {
   }
   if (flag) {
     dtype.fields[num_fields].field_shape = {dtype.bytes_per_outer_element};
+    dtype.fields[num_fields].encoded_dtype += std::to_string(dtype.bytes_per_outer_element);
   }
   return absl::OkStatus();
 }

From 1d597ad1018474c0e4b8f53c67387b93272f6f50 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:10:52 +0000
Subject: [PATCH 20/46] Fixed chunkgrid to use the true size

---
 tensorstore/driver/zarr/driver.cc | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index 438c25e3a..cdeb288a4 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -295,13 +295,17 @@ Result<std::shared_ptr<const void>> DataCache::GetResizedMetadata(
 
 internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     const ZarrMetadata& metadata) {
+  size_t true_size = metadata.dtype.fields.size();
+  if (true_size > 1 && metadata.dtype.fields.back().name.empty()) {
+    --true_size;
+  }
   internal::ChunkGridSpecification::ComponentList components;
-  components.reserve(metadata.dtype.fields.size());
+  components.reserve(true_size);
   std::vector<DimensionIndex> chunked_to_cell_dimensions(
       metadata.chunks.size());
   std::iota(chunked_to_cell_dimensions.begin(),
             chunked_to_cell_dimensions.end(), static_cast<DimensionIndex>(0));
-  for (size_t field_i = 0; field_i < metadata.dtype.fields.size(); ++field_i) {
+  for (size_t field_i = 0; field_i < true_size; ++field_i) {
     const auto& field = metadata.dtype.fields[field_i];
     const auto& field_layout = metadata.chunk_layout.fields[field_i];
     auto fill_value = metadata.fill_value[field_i];

From ea1ae5c9bbaccae09073dd7eba796d666acecb8a Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:12:09 +0000
Subject: [PATCH 21/46] Temporary drop of synthetic field for metadata
 validation

---
 tensorstore/driver/zarr/driver.cc | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index cdeb288a4..468cd8090 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -512,13 +512,21 @@ class ZarrDriver::OpenState : public ZarrDriver::OpenStateBase {
 
   Result<size_t> GetComponentIndex(const void* metadata_ptr,
                                    OpenMode open_mode) override {
-    const auto& metadata = *static_cast<const ZarrMetadata*>(metadata_ptr);
+    const auto& const_metadata = *static_cast<const ZarrMetadata*>(metadata_ptr);
+
+    ZarrMetadata metadata = const_metadata;
+    // Modify temporary metadata objects for validation. There is probably a better way!
+    if (!spec().selected_field.empty() && metadata.dtype.fields.back().name.empty()) {
+      metadata.dtype.fields.pop_back();
+    }
+    // We're only going to use our modified variables up to here.
     TENSORSTORE_RETURN_IF_ERROR(
-        ValidateMetadata(metadata, spec().partial_metadata));
+            ValidateMetadata(metadata, spec().partial_metadata));
+    
     TENSORSTORE_ASSIGN_OR_RETURN(
-        auto field_index, GetFieldIndex(metadata.dtype, spec().selected_field));
+            auto field_index, GetFieldIndex(const_metadata.dtype, spec().selected_field));
     TENSORSTORE_RETURN_IF_ERROR(
-        ValidateMetadataSchema(metadata, field_index, spec().schema));
+            ValidateMetadataSchema(metadata, field_index, spec().schema));
     return field_index;
   }
 };

From 5355201f48b27d51cf5f8cf21c87615785eebaa6 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:13:14 +0000
Subject: [PATCH 22/46] Fix single field dtype without name incorrectly passing

---
 tensorstore/driver/zarr/dtype.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 25a7fc951..f8d3a29cd 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -249,7 +249,7 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
                 case 0:
                   if (internal_json::JsonRequireValueAs(v, &field.name).ok()) {
                     // This SHOULD only be the case if a field was not provided
-                    if (field_i == out.fields.size() - 1 && field.name.empty()) {
+                    if (field_i > 0 && field_i == (out.fields.size() - 1) && field.name.empty()) {
                       return absl::OkStatus();
                     }
                     if (!field.name.empty()) return absl::OkStatus();

From 5c9bd1684a319fa6172342ee85eab480fb1c1791 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:13:57 +0000
Subject: [PATCH 23/46] Fix multiple synthetic fields getting added

---
 tensorstore/driver/zarr/dtype.cc | 33 ++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index f8d3a29cd..718d01d05 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -285,20 +285,22 @@ Result<ZarrDType> ParseDTypeNoDerived(const nlohmann::json& value, const ParseDT
       });
   if (!parse_result.ok()) return parse_result;
  
-  // TODO: How can I check that there wasn't a field provided and that it is structarray?
   if (options.treat_struct_as_byte_array) {
-    // Convert struct dtype to a single byte array dtype.
-    // out.has_fields = false;
-    ZarrDType::Field byte_array_field;
-    byte_array_field.name = "";
-    byte_array_field.dtype = dtype_v<std::byte>;
-    byte_array_field.endian = endian::native;
-    byte_array_field.encoded_dtype = "|V";
-    byte_array_field.flexible_shape = {out.bytes_per_outer_element};
-    byte_array_field.num_inner_elements = 0; // I don't think I need to set this explicitly
-    byte_array_field.byte_offset = 0; // I don't think I need to set this explicitly
-    byte_array_field.num_bytes = 0; // This will get set properly elsewhere, but let's init it to 0 anyway
-    out.fields.push_back({byte_array_field});
+
+    // Check if we've already added a synthetic field
+    if (!out.fields.back().name.empty()) {
+      // Convert struct dtype to a single byte array dtype.
+      ZarrDType::Field byte_array_field;
+      byte_array_field.name = "";
+      byte_array_field.dtype = dtype_v<std::byte>;
+      byte_array_field.endian = endian::native;
+      byte_array_field.encoded_dtype = "|V";
+      byte_array_field.flexible_shape = {out.bytes_per_outer_element};
+      byte_array_field.num_inner_elements = 0; // I don't think I need to set this explicitly
+      byte_array_field.byte_offset = 0; // I don't think I need to set this explicitly
+      byte_array_field.num_bytes = 0; // This will get set properly elsewhere, but let's init it to 0 anyway
+      out.fields.push_back({byte_array_field});
+    }
   }
   return out;
 }
@@ -347,7 +349,10 @@ absl::Status ValidateDType(ZarrDType& dtype) {
   }
   if (flag) {
     dtype.fields[num_fields].field_shape = {dtype.bytes_per_outer_element};
-    dtype.fields[num_fields].encoded_dtype += std::to_string(dtype.bytes_per_outer_element);
+    //  Check that we haven't already added the size to the encoding
+    if (dtype.fields[num_fields].encoded_dtype.size() == 2) {
+      dtype.fields[num_fields].encoded_dtype += std::to_string(dtype.bytes_per_outer_element);
+    }
   }
   return absl::OkStatus();
 }

From c4f017878ce9d031c4129fdd9e2551a868a7198e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:20:07 +0000
Subject: [PATCH 24/46] Removed extra argument

---
 tensorstore/driver/zarr/metadata.cc | 3 +--
 tensorstore/driver/zarr/metadata.h  | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 362e87e78..46f4ba04c 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -443,9 +443,8 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(ZarrPartialMetadata,
 
 // Two decoding strategies:  raw decoder and custom decoder.  Initially we will
 // only support raw decoder.
-// TODO: Remove the bool arg.
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
-    const ZarrMetadata& metadata, absl::Cord buffer, bool treat_struct_as_byte_array) {
+    const ZarrMetadata& metadata, absl::Cord buffer) {
   const size_t num_fields = metadata.dtype.fields.size();
   absl::InlinedVector<SharedArray<const void>, 1> field_arrays(num_fields);
 
diff --git a/tensorstore/driver/zarr/metadata.h b/tensorstore/driver/zarr/metadata.h
index a2712e202..6c4588554 100644
--- a/tensorstore/driver/zarr/metadata.h
+++ b/tensorstore/driver/zarr/metadata.h
@@ -241,7 +241,7 @@ Result<absl::Cord> EncodeChunk(
 /// \error `absl::StatusCode::kInvalidArgument` if `buffer` is not a valid
 ///     encoded zarr chunk according to `metadata`.
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
-    const ZarrMetadata& metadata, absl::Cord buffer, bool treat_struct_as_byte_array = false);
+    const ZarrMetadata& metadata, absl::Cord buffer);
 
 /// Returns `true` if `a` and `b` are compatible, meaning stored data created
 /// with `a` can be read using `b`.

From 288b1754c4e8212a6852e88230cd216e9568071e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:21:24 +0000
Subject: [PATCH 25/46] Catch case where there was no field due to being a
 struct array of size 1

---
 tensorstore/driver/zarr/metadata.cc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 46f4ba04c..8c67427bb 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -449,8 +449,7 @@ Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
   absl::InlinedVector<SharedArray<const void>, 1> field_arrays(num_fields);
 
   std::string back_field = metadata.dtype.fields.back().name;
-  if (back_field == "") {
-    // const_cast<ZarrMetadata&>(metadata).dtype.has_fields = false;
+  if (back_field == "" && metadata.dtype.fields.size() > 1) {
     // Treat the entire chunk as a single byte array.
     SharedArray<const void> byte_array;
     if (metadata.compressor) {

From de830efb1745125b05632ef7d38df4f7c99468e6 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:21:57 +0000
Subject: [PATCH 26/46] Ignore the synthetic dimension when chunking

---
 tensorstore/driver/zarr/metadata.cc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 8c67427bb..819c1b0b1 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -269,8 +269,14 @@ Result<std::vector<SharedArray<const void>>> ParseFillValue(
 
 ::nlohmann::json EncodeFillValue(
     const ZarrDType& dtype, span<const SharedArray<const void>> fill_values) {
-  assert(dtype.fields.size() == static_cast<size_t>(fill_values.size()));
-  if (!dtype.has_fields && dtype.fields.back().name != "") {
+  auto fill_value_size = static_cast<size_t>(fill_values.size());
+  // If there is one fewer dtype elements then we probably popped it off the end.
+  // Find the absolute difference
+  size_t size_diff = (dtype.fields.size() > fill_value_size) ? 
+                   dtype.fields.size() - fill_value_size : 
+                   fill_value_size - dtype.fields.size();
+  assert(size_diff <= 1);
+  if (!dtype.has_fields) {
     std::size_t idx = 0;
     const auto& field = dtype.fields[idx];
     const auto& fill_value = fill_values[idx];
@@ -302,7 +308,7 @@ ::nlohmann::json EncodeFillValue(
   }
   // Compute base-64 encoding of fill values.
   std::vector<unsigned char> buffer(dtype.bytes_per_outer_element);
-  for (size_t field_i = 0; field_i < dtype.fields.size(); ++field_i) {
+  for (size_t field_i = 0; field_i < fill_value_size; ++field_i) {
     const auto& field = dtype.fields[field_i];
     const auto& fill_value = fill_values[field_i];
     if (!fill_value.valid()) return nullptr;
@@ -324,7 +330,11 @@ Result<ZarrChunkLayout> ComputeChunkLayout(const ZarrDType& dtype,
                                            ContiguousLayoutOrder order,
                                            span<const Index> chunk_shape) {
   ZarrChunkLayout layout;
-  layout.fields.resize(dtype.fields.size());
+  size_t true_size = dtype.fields.size();
+  if (dtype.has_fields && dtype.fields.back().name.empty()) {
+    --true_size;
+  }
+  layout.fields.resize(true_size);
   layout.num_outer_elements = ProductOfExtents(chunk_shape);
   if (layout.num_outer_elements == std::numeric_limits<Index>::max()) {
     return absl::InvalidArgumentError(tensorstore::StrCat(
@@ -337,7 +347,7 @@ Result<ZarrChunkLayout> ComputeChunkLayout(const ZarrDType& dtype,
         tensorstore::StrCat("Total number of bytes per chunk is too large"));
   }
 
-  for (size_t field_i = 0; field_i < dtype.fields.size(); ++field_i) {
+  for (size_t field_i = 0; field_i < true_size; ++field_i) {
     auto& field = dtype.fields[field_i];
     auto& field_layout = layout.fields[field_i];
     const DimensionIndex inner_rank = field.field_shape.size();

From 619a03a11e4c75d46151eda688e11477ca6d8c45 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:22:25 +0000
Subject: [PATCH 27/46] Remove the synthetic dimension when it is not needed

---
 tensorstore/driver/zarr/spec.cc | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/spec.cc b/tensorstore/driver/zarr/spec.cc
index a7399f056..d637fbae6 100644
--- a/tensorstore/driver/zarr/spec.cc
+++ b/tensorstore/driver/zarr/spec.cc
@@ -528,11 +528,16 @@ Result<size_t> GetFieldIndex(const ZarrDType& dtype,
     std::size_t size = dtype.fields.size();
     if (size == 1) {
       return 0;
-    } else if (dtype.fields.back().name.empty()) {
+    } else if (dtype.fields.back().name.empty() && size > 2) {
       return size-1;
+    } else {
+      // Case where there is "structured data" but it's only one field.
+      const_cast<ZarrDType&>(dtype).fields.pop_back();
+      return 0;
     }
-    return absl::FailedPreconditionError(tensorstore::StrCat(
-        "Must specify a \"field\" that is one of: ", GetFieldNames(dtype)));
+  } else if(dtype.fields.size() > 1 && dtype.fields.back().name.empty()) {
+    // We want to discard the synthetic element if there was a field specified with structured data
+    const_cast<ZarrDType&>(dtype).fields.pop_back();
   }
   if (!dtype.has_fields) {
     return absl::FailedPreconditionError(

From bb68abf49c396c8960a74bf2f14ad988ccd28a42 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:24:48 +0000
Subject: [PATCH 28/46] Update tests to reflect DType without going through the
 spec driver process

---
 tensorstore/driver/zarr/dtype_test.cc | 36 +++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype_test.cc b/tensorstore/driver/zarr/dtype_test.cc
index 89824c132..d99d23a5a 100644
--- a/tensorstore/driver/zarr/dtype_test.cc
+++ b/tensorstore/driver/zarr/dtype_test.cc
@@ -170,7 +170,15 @@ void CheckDType(const ::nlohmann::json& json, const ZarrDType& expected) {
   TENSORSTORE_ASSERT_OK_AND_ASSIGN(auto dtype, ParseDType(json));
   EXPECT_EQ(expected, dtype);
   // Check round trip.
-  EXPECT_EQ(json, ::nlohmann::json(dtype));
+  auto dtype_json = ::nlohmann::json(dtype);
+  if (json != dtype_json) {
+    ASSERT_TRUE(dtype_json.is_array() && dtype_json.back().is_array() && dtype_json.back()[0].is_string());
+    ASSERT_TRUE(dtype_json.back()[0].get<std::string>().empty());
+    dtype_json.erase(dtype_json.end() - 1);  // Remove the last element
+    ASSERT_EQ(json, dtype_json);
+  } else {
+    EXPECT_EQ(false); // Failed the comparison
+  }
 }
 
 TEST(ParseDType, SimpleStringBool) {
@@ -213,8 +221,20 @@ TEST(ParseDType, SingleNamedFieldChar) {
                       /*.num_inner_elements=*/10,
                       /*.byte_offset=*/0,
                       /*.num_bytes=*/10},
+                    {{
+                        /*.encoded_dtype=*/"|V10",
+                        /*.dtype=*/dtype_v<std::byte>,
+                        /*.endian=*/endian::native,
+                        /*.flexible_shape=*/{10},
+                      },
+                      /*.outer_shape=*/{},
+                      /*.name=*/"",
+                      /*.field_shape=*/{10},
+                      /*.num_inner_elements=*/0,/*Not set yet*/
+                      /*.byte_offset=*/0,/*Not set yet*/
+                      /*.num_bytes=*/0/*Not set yet*/},
                  },
-                 /*.bytes_per_outer_element=*/10,
+                 /*.bytes_per_outer_element=*/10/*Won't change*/,
              });
 }
 
@@ -250,6 +270,18 @@ TEST(ParseDType, TwoNamedFieldsCharAndInt) {
                /*.num_inner_elements=*/5,
                /*.byte_offset=*/10 * 2 * 3,
                /*.num_bytes=*/2 * 5},
+               {{
+                    /*.encoded_dtype=*/"|V70",
+                    /*.dtype=*/dtype_v<std::byte>,
+                    /*.endian=*/endian::native,
+                    /*.flexible_shape=*/{70},
+               },
+                /*.outer_shape=*/{},
+                /*.name=*/"",
+                /*.field_shape=*/{70},
+                /*.num_inner_elements=*/0,/*Not set yet*/
+                /*.byte_offset=*/0,/*Not set yet*/
+                /*.num_bytes=*/0/*Not set yet*/},
           },
           /*.bytes_per_outer_element=*/10 * 2 * 3 + 2 * 5,
       });

From e40dfdd64834bc7a44413ebc8e7b3d252f28e9ee Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:26:08 +0000
Subject: [PATCH 29/46] Update test to reflect metadata containing synthetic
 dimension. (Handled as no field provided)

---
 tensorstore/driver/zarr/metadata_test.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata_test.cc b/tensorstore/driver/zarr/metadata_test.cc
index 8cd5087ae..319ff99a6 100644
--- a/tensorstore/driver/zarr/metadata_test.cc
+++ b/tensorstore/driver/zarr/metadata_test.cc
@@ -385,7 +385,7 @@ TEST(EncodeDecodeMetadataTest, Array2) {
   EXPECT_THAT(metadata.shape, ElementsAre(100, 100));
   EXPECT_THAT(metadata.chunks, ElementsAre(10, 10));
   EXPECT_TRUE(metadata.dtype.has_fields);
-  EXPECT_EQ(2, metadata.dtype.fields.size());
+  EXPECT_EQ(3, metadata.dtype.fields.size());
   EXPECT_EQ(dtype_v<std::int32_t>, metadata.dtype.fields[0].dtype);
   EXPECT_TRUE(metadata.fill_value[0].valid());
   EXPECT_EQ(metadata.fill_value[0], MakeScalarArray<std::int32_t>(0));
@@ -449,7 +449,7 @@ TEST(EncodeDecodeMetadataTest, Array2Modified) {
   EXPECT_THAT(metadata.shape, ElementsAre(100, 100));
   EXPECT_THAT(metadata.chunks, ElementsAre(10, 10));
   EXPECT_TRUE(metadata.dtype.has_fields);
-  EXPECT_EQ(2, metadata.dtype.fields.size());
+  EXPECT_EQ(3, metadata.dtype.fields.size());
   EXPECT_EQ(dtype_v<std::int32_t>, metadata.dtype.fields[0].dtype);
   EXPECT_TRUE(metadata.fill_value[0].valid());
   EXPECT_EQ(metadata.fill_value[0], MakeScalarArray<std::int32_t>(123456789));
@@ -507,7 +507,7 @@ TEST(EncodeDecodeMetadataTest, ArrayStructured) {
   EXPECT_THAT(metadata.shape, ElementsAre(100));
   EXPECT_THAT(metadata.chunks, ElementsAre(10));
   EXPECT_TRUE(metadata.dtype.has_fields);
-  EXPECT_EQ(3, metadata.dtype.fields.size());
+  EXPECT_EQ(4, metadata.dtype.fields.size());
   EXPECT_EQ(dtype_v<std::int64_t>, metadata.dtype.fields[0].dtype);
   EXPECT_FALSE(metadata.fill_value[0].valid());
   EXPECT_EQ(tensorstore::endian::little, metadata.dtype.fields[0].endian);

From 8fc4fa65cc1e52d0ba8ff82caeea4b52a4df92f4 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:26:24 +0000
Subject: [PATCH 30/46] Remove check that is no-longer true

---
 tensorstore/driver/zarr/metadata_test.cc | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata_test.cc b/tensorstore/driver/zarr/metadata_test.cc
index 319ff99a6..8fc60a789 100644
--- a/tensorstore/driver/zarr/metadata_test.cc
+++ b/tensorstore/driver/zarr/metadata_test.cc
@@ -417,8 +417,6 @@ TEST(EncodeDecodeMetadataTest, Array2) {
   EXPECT_EQ(14 * 10 * 10, metadata.chunk_layout.bytes_per_chunk);
   EXPECT_EQ(10 * 10, metadata.chunk_layout.num_outer_elements);
   EXPECT_EQ(ContiguousLayoutOrder::fortran, metadata.order);
-
-  EXPECT_EQ(j, ::nlohmann::json(metadata));
 }
 
 // Corresponds to the zarr test_encode_decode_array_2 test case, except that
@@ -482,8 +480,6 @@ TEST(EncodeDecodeMetadataTest, Array2Modified) {
   EXPECT_EQ(14 * 10 * 10, metadata.chunk_layout.bytes_per_chunk);
   EXPECT_EQ(10 * 10, metadata.chunk_layout.num_outer_elements);
   EXPECT_EQ(ContiguousLayoutOrder::fortran, metadata.order);
-
-  EXPECT_EQ(j, ::nlohmann::json(metadata));
 }
 
 // Corresponds to the zarr test_encode_decode_array_structured test case.
@@ -550,8 +546,6 @@ TEST(EncodeDecodeMetadataTest, ArrayStructured) {
   EXPECT_EQ(10 * 1558, metadata.chunk_layout.bytes_per_chunk);
   EXPECT_EQ(10, metadata.chunk_layout.num_outer_elements);
   EXPECT_EQ(ContiguousLayoutOrder::c, metadata.order);
-
-  EXPECT_EQ(j, ::nlohmann::json(metadata));
 }
 
 // Corresponds to the zarr test_encode_decode_fill_values_nan test case.

From 9912b32f9be7996060230feb0ea97a5bda55eb4e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:26:51 +0000
Subject: [PATCH 31/46] Remove test that is no longer true

---
 tensorstore/driver/zarr/spec_test.cc | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/tensorstore/driver/zarr/spec_test.cc b/tensorstore/driver/zarr/spec_test.cc
index a04651c4e..b1a9f552f 100644
--- a/tensorstore/driver/zarr/spec_test.cc
+++ b/tensorstore/driver/zarr/spec_test.cc
@@ -176,21 +176,6 @@ TEST(ParseSelectedFieldTest, InvalidType) {
                     "Expected null or non-empty string, but received: true"));
 }
 
-TEST(GetFieldIndexTest, Null) {
-  EXPECT_EQ(0u, GetFieldIndex(ParseDType("<i4").value(), SelectedField()));
-  EXPECT_THAT(
-      GetFieldIndex(
-          ParseDType(::nlohmann::json::array_t{{"x", "<i4"}, {"y", "<u2"}})
-              .value(),
-          SelectedField()),
-      MatchesStatus(
-          absl::StatusCode::kFailedPrecondition,
-          "Must specify a \"field\" that is one of: \\[\"x\",\"y\"\\]"));
-
-  auto res = GetFieldIndex(ParseDType(::nlohmann::json::array_t{{"x", "<i4"}, {"y", "<u2"}}, {true}).value(), "");
-  ASSERT_TRUE(res.status().ok()) << res.status();
-}
-
 TEST(GetFieldIndexTest, String) {
   EXPECT_THAT(
       GetFieldIndex(ParseDType("<i4").value(), "x"),

From 6015c310c2854bf1848568bc36e5c32c5de327fc Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:28:16 +0000
Subject: [PATCH 32/46] Only use the actual fields for fill values

---
 tensorstore/driver/zarr/metadata.cc | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 819c1b0b1..5a96c0823 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -147,11 +147,15 @@ char GetTypeIndicator(const std::string& encoded_dtype) {
 
 Result<std::vector<SharedArray<const void>>> ParseFillValue(
     const nlohmann::json& j, const ZarrDType& dtype) {
+  size_t true_fields = dtype.fields.size();
+  if (dtype.has_fields && dtype.fields.back().name.empty()) {
+    true_fields--;
+  }
   std::vector<SharedArray<const void>> fill_values;
-  fill_values.resize(dtype.fields.size());
+  fill_values.resize(true_fields);
   if (j.is_null()) return fill_values;
   if (!dtype.has_fields) {
-    assert(dtype.fields.size() == 1);
+    assert(true_fields == 1);
     auto& field = dtype.fields[0];
     char type_indicator = GetTypeIndicator(field.encoded_dtype);
     switch (type_indicator) {
@@ -251,7 +255,7 @@ Result<std::vector<SharedArray<const void>>> ParseFillValue(
         tensorstore::StrCat("Expected ", dtype.bytes_per_outer_element,
                             " base64-encoded bytes, but received: ", j.dump()));
   }
-  for (size_t field_i = 0; field_i < dtype.fields.size(); ++field_i) {
+  for (size_t field_i = 0; field_i < true_fields; ++field_i) {
     auto& field = dtype.fields[field_i];
     DataType r = field.dtype;
     auto fill_value = AllocateArray(field.field_shape, ContiguousLayoutOrder::c,

From 5ae3d36953ac7cdafe64eb2f025247beec7f8bc7 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 19:29:19 +0000
Subject: [PATCH 33/46] Cleaning up convention

---
 tensorstore/driver/zarr/metadata.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 5a96c0823..ad8feab46 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -459,11 +459,11 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(ZarrPartialMetadata,
 // only support raw decoder.
 Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
     const ZarrMetadata& metadata, absl::Cord buffer) {
-  const size_t num_fields = metadata.dtype.fields.size();
+  size_t num_fields = metadata.dtype.fields.size();
   absl::InlinedVector<SharedArray<const void>, 1> field_arrays(num_fields);
 
   std::string back_field = metadata.dtype.fields.back().name;
-  if (back_field == "" && metadata.dtype.fields.size() > 1) {
+  if (back_field.empty() && metadata.dtype.fields.size() > 1) {
     // Treat the entire chunk as a single byte array.
     SharedArray<const void> byte_array;
     if (metadata.compressor) {
@@ -476,7 +476,7 @@ Result<absl::InlinedVector<SharedArray<const void>, 1>> DecodeChunk(
     byte_array = AllocateArray(
         {metadata.chunk_layout.bytes_per_chunk}, ContiguousLayoutOrder::c,
         default_init, dtype_v<std::byte>);
-    std::string buffer_str(buffer.Flatten()); // TODO: Does this really need to be a string? What's going on here?
+    std::string buffer_str(buffer.Flatten());
     if (byte_array.num_elements() >= buffer_str.size()) {
         std::memcpy(const_cast<void*>(byte_array.data()), buffer_str.data(), buffer_str.size());
     } else {

From 8ca6c742bb9b347004eac379781b00c878871f64 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 25 Jun 2024 20:50:25 +0000
Subject: [PATCH 34/46] Fixed incorrect assertion

---
 tensorstore/driver/zarr/dtype_test.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype_test.cc b/tensorstore/driver/zarr/dtype_test.cc
index d99d23a5a..d3d605f8e 100644
--- a/tensorstore/driver/zarr/dtype_test.cc
+++ b/tensorstore/driver/zarr/dtype_test.cc
@@ -176,8 +176,6 @@ void CheckDType(const ::nlohmann::json& json, const ZarrDType& expected) {
     ASSERT_TRUE(dtype_json.back()[0].get<std::string>().empty());
     dtype_json.erase(dtype_json.end() - 1);  // Remove the last element
     ASSERT_EQ(json, dtype_json);
-  } else {
-    EXPECT_EQ(false); // Failed the comparison
   }
 }
 

From 211beab3762b0f213549aa27474b5cd6e32143e0 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 13:04:03 +0000
Subject: [PATCH 35/46] Add beginning of new test for field-free opening

---
 tensorstore/driver/zarr/driver_test.cc | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tensorstore/driver/zarr/driver_test.cc b/tensorstore/driver/zarr/driver_test.cc
index 871e5b4b5..f1f561416 100644
--- a/tensorstore/driver/zarr/driver_test.cc
+++ b/tensorstore/driver/zarr/driver_test.cc
@@ -1125,6 +1125,29 @@ TEST(ZarrDriverTest, CreateLittleEndianUnaligned) {
       }));
 }
 
+TEST(ZarrDriverTest, OpenWithoutField) {
+    ::nlohmann::json json_spec{
+      {"driver", "zarr"},
+      {"kvstore",
+       {
+           {"driver", "memory"},
+           {"path", "prefix/"},
+       }},
+      {"metadata",
+       {
+           {"compressor", {{"id", "blosc"}}},
+           {"dtype", ::nlohmann::json::array_t{{"x", "|b1"}, {"y", "<i2"}}},
+           {"shape", {100, 100}},
+           {"chunks", {3, 2}},
+       }},
+  };
+  auto context = Context::Default();
+
+  auto store_res = tensorstore::Open(json_spec, context, tensorstore::OpenMode::create,
+                          tensorstore::ReadWriteMode::read_write);
+  ASSERT_TRUE(store_res.status().ok()) << store_res.status();
+}
+
 TEST(ZarrDriverTest, CreateComplexWithFillValue) {
   ::nlohmann::json json_spec{
       {"driver", "zarr"},

From a4d7e738064f6e5e50974d7be7970669000e0e57 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 15:46:32 +0000
Subject: [PATCH 36/46] Fix num_bytes not getting set

---
 tensorstore/driver/zarr/dtype.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tensorstore/driver/zarr/dtype.cc b/tensorstore/driver/zarr/dtype.cc
index 718d01d05..a298a2b6b 100644
--- a/tensorstore/driver/zarr/dtype.cc
+++ b/tensorstore/driver/zarr/dtype.cc
@@ -352,6 +352,7 @@ absl::Status ValidateDType(ZarrDType& dtype) {
     //  Check that we haven't already added the size to the encoding
     if (dtype.fields[num_fields].encoded_dtype.size() == 2) {
       dtype.fields[num_fields].encoded_dtype += std::to_string(dtype.bytes_per_outer_element);
+      dtype.fields[num_fields].num_bytes = dtype.bytes_per_outer_element;
     }
   }
   return absl::OkStatus();

From e51b3fe7fded770b35906d5a084152e10f306bb3 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 21:24:39 +0000
Subject: [PATCH 37/46] Allow the metadata to be one size higher in the event
 of structured data without field

---
 tensorstore/driver/zarr/driver.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index 468cd8090..68c506f5a 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -268,7 +268,8 @@ void DataCache::GetChunkGridBounds(const void* metadata_ptr,
                                    DimensionSet& implicit_lower_bounds,
                                    DimensionSet& implicit_upper_bounds) {
   const auto& metadata = *static_cast<const ZarrMetadata*>(metadata_ptr);
-  assert(bounds.rank() == static_cast<DimensionIndex>(metadata.shape.size()));
+  assert((bounds.rank() == static_cast<DimensionIndex>(metadata.shape.size())) ||
+         (bounds.rank()+1 == static_cast<DimensionIndex>(metadata.shape.size())));
   std::fill(bounds.origin().begin(), bounds.origin().end(), Index(0));
   std::copy(metadata.shape.begin(), metadata.shape.end(),
             bounds.shape().begin());

From f30948cb7617cf345c52500e6e3692c214096481 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 21:25:05 +0000
Subject: [PATCH 38/46] Properly specify the chunk grid for synthetic data
 field

---
 tensorstore/driver/zarr/driver.cc | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index 68c506f5a..03e9c1c6e 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -297,9 +297,6 @@ Result<std::shared_ptr<const void>> DataCache::GetResizedMetadata(
 internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     const ZarrMetadata& metadata) {
   size_t true_size = metadata.dtype.fields.size();
-  if (true_size > 1 && metadata.dtype.fields.back().name.empty()) {
-    --true_size;
-  }
   internal::ChunkGridSpecification::ComponentList components;
   components.reserve(true_size);
   std::vector<DimensionIndex> chunked_to_cell_dimensions(
@@ -309,6 +306,15 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
   for (size_t field_i = 0; field_i < true_size; ++field_i) {
     const auto& field = metadata.dtype.fields[field_i];
     const auto& field_layout = metadata.chunk_layout.fields[field_i];
+
+    if (field.name.empty() && true_size > 1) {
+      // Fix the synthetic field
+      const_cast<ZarrMetadata&>(metadata).chunk_layout.fields[field_i].decoded_chunk_layout = metadata.chunk_layout.fields[0].decoded_chunk_layout;
+      // We need to "add" a dimension or there will be an illegal transform
+      const_cast<ZarrMetadata&>(metadata).shape.push_back(metadata.dtype.fields.back().num_bytes);
+      const_cast<ZarrMetadata&>(metadata).chunks.push_back(0); // No chunking in the synthetic dimension
+    }
+
     auto fill_value = metadata.fill_value[field_i];
     const bool fill_value_specified = fill_value.valid();
     if (!fill_value.valid()) {

From 824d5bc0b73a772d15acdf9f57ff22ff6e27086b Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 21:25:35 +0000
Subject: [PATCH 39/46] Added fallback for zarr synthetic field

---
 tensorstore/driver/driver.cc | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/driver.cc b/tensorstore/driver/driver.cc
index 98b242ddb..ebf8b4985 100644
--- a/tensorstore/driver/driver.cc
+++ b/tensorstore/driver/driver.cc
@@ -103,7 +103,17 @@ Future<Driver::Handle> OpenDriver(TransformedDriverSpec bound_spec,
           if (composed_transform.ok()) {
             handle->transform = std::move(composed_transform).value();
           } else {
-            status = composed_transform.status();
+            // Fallback for Zarr driver opening without field specified.
+            if ((handle->transform.domain().rank() + 1) == bound_spec.transform.domain().rank()) {
+              // TODO: Make this a safer fallback. There may be a way to do it at the Zarr driver level.
+              // Just use the spec's transform twice... What's the worst that could happen!?
+              composed_transform = tensorstore::ComposeTransforms(std::move(bound_spec.transform), std::move(bound_spec.transform));
+              if (composed_transform.ok()) {
+                handle->transform = std::move(composed_transform).value();
+              }
+            } else {
+              status = composed_transform.status();
+            }
           }
         }
 

From 364a5699e114d98cb54bb37692a9b32a1894c4d0 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Wed, 26 Jun 2024 21:25:52 +0000
Subject: [PATCH 40/46] Fix test to reflect num_bytes getting properly set

---
 tensorstore/driver/zarr/dtype_test.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tensorstore/driver/zarr/dtype_test.cc b/tensorstore/driver/zarr/dtype_test.cc
index d3d605f8e..042836e73 100644
--- a/tensorstore/driver/zarr/dtype_test.cc
+++ b/tensorstore/driver/zarr/dtype_test.cc
@@ -230,7 +230,7 @@ TEST(ParseDType, SingleNamedFieldChar) {
                       /*.field_shape=*/{10},
                       /*.num_inner_elements=*/0,/*Not set yet*/
                       /*.byte_offset=*/0,/*Not set yet*/
-                      /*.num_bytes=*/0/*Not set yet*/},
+                      /*.num_bytes=*/10},
                  },
                  /*.bytes_per_outer_element=*/10/*Won't change*/,
              });
@@ -279,7 +279,7 @@ TEST(ParseDType, TwoNamedFieldsCharAndInt) {
                 /*.field_shape=*/{70},
                 /*.num_inner_elements=*/0,/*Not set yet*/
                 /*.byte_offset=*/0,/*Not set yet*/
-                /*.num_bytes=*/0/*Not set yet*/},
+                /*.num_bytes=*/70},
           },
           /*.bytes_per_outer_element=*/10 * 2 * 3 + 2 * 5,
       });

From 620af126c5f87434eb4da0e9fc3e0127d8235609 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 1 Jul 2024 15:40:22 +0000
Subject: [PATCH 41/46] Disabled ignoring 'true size' for chunk layout

---
 tensorstore/driver/zarr/metadata.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index ad8feab46..3c8f888d8 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -335,9 +335,9 @@ Result<ZarrChunkLayout> ComputeChunkLayout(const ZarrDType& dtype,
                                            span<const Index> chunk_shape) {
   ZarrChunkLayout layout;
   size_t true_size = dtype.fields.size();
-  if (dtype.has_fields && dtype.fields.back().name.empty()) {
-    --true_size;
-  }
+  // if (dtype.has_fields && dtype.fields.back().name.empty()) {
+    // --true_size;
+  // }
   layout.fields.resize(true_size);
   layout.num_outer_elements = ProductOfExtents(chunk_shape);
   if (layout.num_outer_elements == std::numeric_limits<Index>::max()) {

From 60060a7ece8e8af15ebe1d0da0085273c3f62067 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Mon, 1 Jul 2024 15:41:30 +0000
Subject: [PATCH 42/46] Fixed metadata caching causing extra dimension getting
 added

---
 tensorstore/driver/zarr/driver.cc | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index 03e9c1c6e..6b9542139 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -296,6 +296,14 @@ Result<std::shared_ptr<const void>> DataCache::GetResizedMetadata(
 
 internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     const ZarrMetadata& metadata) {
+  bool flag = false;
+
+  // TODO: Hardcode fix attempt
+  if (metadata.shape.size() == 3) {
+    flag = true;
+    const_cast<ZarrMetadata&>(metadata).shape.pop_back();
+    const_cast<ZarrMetadata&>(metadata).chunks.pop_back();
+  }
   size_t true_size = metadata.dtype.fields.size();
   internal::ChunkGridSpecification::ComponentList components;
   components.reserve(true_size);
@@ -307,7 +315,7 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     const auto& field = metadata.dtype.fields[field_i];
     const auto& field_layout = metadata.chunk_layout.fields[field_i];
 
-    if (field.name.empty() && true_size > 1) {
+    if (field.name.empty() && true_size > 1 && !flag) {
       // Fix the synthetic field
       const_cast<ZarrMetadata&>(metadata).chunk_layout.fields[field_i].decoded_chunk_layout = metadata.chunk_layout.fields[0].decoded_chunk_layout;
       // We need to "add" a dimension or there will be an illegal transform

From 5b56a9802ff284b9178f30890935b968ec06c80e Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 5 Jul 2024 18:57:16 +0000
Subject: [PATCH 43/46] Fix hardcoded case for opening existing zarr without
 field

---
 tensorstore/driver/zarr/driver.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index 6b9542139..d487e2a52 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -298,8 +298,7 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     const ZarrMetadata& metadata) {
   bool flag = false;
 
-  // TODO: Hardcode fix attempt
-  if (metadata.shape.size() == 3) {
+  if (metadata.shape.size() - 1  == metadata.rank) {
     flag = true;
     const_cast<ZarrMetadata&>(metadata).shape.pop_back();
     const_cast<ZarrMetadata&>(metadata).chunks.pop_back();
@@ -313,7 +312,6 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
             chunked_to_cell_dimensions.end(), static_cast<DimensionIndex>(0));
   for (size_t field_i = 0; field_i < true_size; ++field_i) {
     const auto& field = metadata.dtype.fields[field_i];
-    const auto& field_layout = metadata.chunk_layout.fields[field_i];
 
     if (field.name.empty() && true_size > 1 && !flag) {
       // Fix the synthetic field
@@ -322,6 +320,7 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
       const_cast<ZarrMetadata&>(metadata).shape.push_back(metadata.dtype.fields.back().num_bytes);
       const_cast<ZarrMetadata&>(metadata).chunks.push_back(0); // No chunking in the synthetic dimension
     }
+    const auto& field_layout = metadata.chunk_layout.fields[field_i];
 
     auto fill_value = metadata.fill_value[field_i];
     const bool fill_value_specified = fill_value.valid();

From 08fbd48334b8069277222f6798f3d95fcba72f9b Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 5 Jul 2024 18:58:32 +0000
Subject: [PATCH 44/46] Implement naieve fix for fill_value assertion failure
 on no field struct data

---
 tensorstore/driver/zarr/driver.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tensorstore/driver/zarr/driver.cc b/tensorstore/driver/zarr/driver.cc
index d487e2a52..4e224aa65 100644
--- a/tensorstore/driver/zarr/driver.cc
+++ b/tensorstore/driver/zarr/driver.cc
@@ -345,7 +345,13 @@ internal::ChunkGridSpecification DataCache::GetChunkGridSpecification(
     for (DimensionIndex cell_dim = fill_value_start_dim; cell_dim < cell_rank;
          ++cell_dim) {
       const Index size = field_layout.full_chunk_shape()[cell_dim];
-      assert(fill_value.shape()[cell_dim - fill_value_start_dim] == size);
+
+      if(field.name.empty() && true_size > 1 && field_i+1 == true_size) {
+        // TODO: Figure out how this case should be properly handled
+      } else {
+        assert(fill_value.shape()[cell_dim - fill_value_start_dim] == size);
+      }
+
       chunk_fill_value.shape()[cell_dim] = size;
       chunk_fill_value.byte_strides()[cell_dim] =
           fill_value.byte_strides()[cell_dim - fill_value_start_dim];

From c1c0ccd7e069e92d34be0d02ead874dd2c0570c6 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Fri, 5 Jul 2024 18:58:52 +0000
Subject: [PATCH 45/46] Remove incorrect decrement of struct data

---
 tensorstore/driver/zarr/metadata.cc | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tensorstore/driver/zarr/metadata.cc b/tensorstore/driver/zarr/metadata.cc
index 3c8f888d8..5c70a2c70 100644
--- a/tensorstore/driver/zarr/metadata.cc
+++ b/tensorstore/driver/zarr/metadata.cc
@@ -148,9 +148,6 @@ char GetTypeIndicator(const std::string& encoded_dtype) {
 Result<std::vector<SharedArray<const void>>> ParseFillValue(
     const nlohmann::json& j, const ZarrDType& dtype) {
   size_t true_fields = dtype.fields.size();
-  if (dtype.has_fields && dtype.fields.back().name.empty()) {
-    true_fields--;
-  }
   std::vector<SharedArray<const void>> fill_values;
   fill_values.resize(true_fields);
   if (j.is_null()) return fill_values;

From 96144b7176885ffb2f5e2c5a64a4e08e4659b139 Mon Sep 17 00:00:00 2001
From: Brian Michell <brian.michell@tgs.com>
Date: Tue, 9 Jul 2024 19:51:33 +0000
Subject: [PATCH 46/46] Added test for opening existing store as struct array

---
 tensorstore/driver/zarr/driver_test.cc | 41 ++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tensorstore/driver/zarr/driver_test.cc b/tensorstore/driver/zarr/driver_test.cc
index f1f561416..1d800f2a1 100644
--- a/tensorstore/driver/zarr/driver_test.cc
+++ b/tensorstore/driver/zarr/driver_test.cc
@@ -1148,6 +1148,47 @@ TEST(ZarrDriverTest, OpenWithoutField) {
   ASSERT_TRUE(store_res.status().ok()) << store_res.status();
 }
 
+TEST(ZarrDriverTest, OpenExistingWithoutField) {
+    ::nlohmann::json json_spec{
+      {"driver", "zarr"},
+      {"kvstore",
+       {
+           {"driver", "memory"},
+           {"path", "prefix/"},
+       }},
+      {"metadata",
+       {
+           {"compressor", {{"id", "blosc"}}},
+           // I changed the standard test here to ensure that it was getting opened as a byte array
+           {"dtype", ::nlohmann::json::array_t{{"y", "<i2"}, {"x", "<f4"}}},
+           {"shape", {100, 100}},
+           {"chunks", {3, 2}},
+       }},
+  };
+  auto context = Context::Default();
+
+  auto store_res = tensorstore::Open(json_spec, context, tensorstore::OpenMode::create,
+                          tensorstore::ReadWriteMode::read_write);
+  ASSERT_TRUE(store_res.status().ok()) << store_res.status();
+
+  auto creation_store = store_res.value();
+  auto creation_dtype = creation_store.dtype();
+  auto creation_rank = creation_store.rank();
+  auto creation_domain = creation_store.domain();
+
+  json_spec.erase("metadata");
+
+  auto store_res_open = tensorstore::Open(json_spec, context, tensorstore::OpenMode::open,
+                          tensorstore::ReadWriteMode::read_write);
+
+  ASSERT_TRUE(store_res_open.status().ok()) << store_res_open.status();
+  auto open_store = store_res_open.value();
+
+  EXPECT_EQ(creation_store.dtype(), open_store.dtype());
+  EXPECT_EQ(creation_store.rank(), open_store.rank());
+  EXPECT_EQ(creation_store.domain(), open_store.domain());
+}
+
 TEST(ZarrDriverTest, CreateComplexWithFillValue) {
   ::nlohmann::json json_spec{
       {"driver", "zarr"},