Remove implicit group support (#51)

LDeakin · Aug 30, 2024 · 5256086 · 5256086
1 parent 489f357
commit 5256086
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 71 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  - **Breaking**: Make `create_chunk_grid_{regular,rectangular}` `pub(crate)` in alignment with other internal create from metadata methods
  - **Breaking**: Bump MSRV to 1.76 (8 February, 2024)
  - [#59](https://github.com/LDeakin/zarrs/pull/59) Add `ReadableWritableStorageTraits` automatically for all implementations by [@dustinlagoy]
+ - Remove implicit group support
+   - This is a post-acceptance change of Zarr V3: https://github.com/zarr-developers/zarr-specs/pull/292
 
 ### Fixed
  - `[async_]store_set_partial_values` no longer truncates

diff --git a/src/group.rs b/src/group.rs
@@ -1,12 +1,16 @@
 //! Zarr groups.
 //!
 //! A Zarr group is a node in a Zarr hierarchy.
-//! It can have associated metadata and may have child nodes (groups or [`arrays`](crate::array)).
+//! It can have associated attributes and may have child nodes (groups or [`arrays`](crate::array)).
 //! See <https://zarr-specs.readthedocs.io/en/latest/v3/core/v3.0.html#group>.
 //!
 //! Use [`GroupBuilder`] to setup a new group, or use [`Group::open`] to read and/or write an existing group.
 //!
-//! A group can optionally store attributes in metadata in an accompanying `zarr.json` file. For example:
+//! ## Group Metadata
+//! Group metadata **must be explicitly stored** with [`store_metadata`](Group::store_metadata) or [`store_metadata_opt`](Group::store_metadata_opt) if a group is newly created or its metadata has been mutated.
+//! Support for implicit groups was removed from Zarr V3 after provisional acceptance.
+//!
+//! Below is an example of a `zarr.json` file for a group:
 //! ```json
 //! {
 //!     "zarr_format": 3,
@@ -207,20 +211,7 @@ impl<TStorage: ?Sized + ReadableStorageTraits> Group<TStorage> {
         }
 
         // No metadata has been found
-        match version {
-            MetadataRetrieveVersion::Default | MetadataRetrieveVersion::V3 => {
-                // V3 supports missing metadata
-                Self::new_with_metadata(
-                    storage,
-                    path,
-                    GroupMetadata::V3(GroupMetadataV3::default()),
-                )
-            }
-            MetadataRetrieveVersion::V2 => {
-                // V2 does not support missing metadata
-                Err(GroupCreateError::MissingMetadata)
-            }
-        }
+        Err(GroupCreateError::MissingMetadata)
     }
 }
 
@@ -276,20 +267,7 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits> Group<TStorage> {
         }
 
         // No metadata has been found
-        match version {
-            MetadataRetrieveVersion::Default | MetadataRetrieveVersion::V3 => {
-                // V3 supports missing metadata
-                Self::new_with_metadata(
-                    storage,
-                    path,
-                    GroupMetadata::V3(GroupMetadataV3::default()),
-                )
-            }
-            MetadataRetrieveVersion::V2 => {
-                // V2 does not support missing metadata
-                Err(GroupCreateError::MissingMetadata)
-            }
-        }
+        Err(GroupCreateError::MissingMetadata)
     }
 }
 
@@ -305,8 +283,8 @@ pub enum GroupCreateError {
     /// Storage error.
     #[error(transparent)]
     StorageError(#[from] StorageError),
-    /// Missing metadata (Zarr V2 only).
-    #[error("group metadata is missing (Zarr V2 only)")]
+    /// Missing metadata.
+    #[error("group metadata is missing")]
     MissingMetadata,
 }
 
@@ -603,12 +581,11 @@ mod tests {
         assert_eq!(group_copy.metadata(), group.metadata());
     }
 
+    /// Implicit group support is removed since implicit groups were removed from the Zarr V3 spec
     #[test]
-    fn group_default() {
+    fn group_implicit() {
         let store = std::sync::Arc::new(MemoryStore::new());
         let group_path = "/group";
-        let group = Group::open(store, group_path).unwrap();
-        assert_eq!(group.attributes(), &serde_json::Map::default());
-        assert_eq!(group.additional_fields(), &AdditionalFields::default());
+        assert!(Group::open(store, group_path).is_err());
     }
 }
diff --git a/src/node.rs b/src/node.rs
@@ -19,7 +19,6 @@ use thiserror::Error;
 
 use crate::{
     array::ArrayMetadata,
-    group::GroupMetadataV3,
     metadata::{ArrayMetadataV2, GroupMetadata, GroupMetadataV2, MetadataRetrieveVersion},
     storage::{
         get_child_nodes, meta_key, meta_key_v2_array, meta_key_v2_attributes, meta_key_v2_group,
@@ -59,8 +58,8 @@ pub enum NodeCreateError {
     /// Metadata version mismatch
     #[error("Found V2 metadata in V3 key or vice-versa")]
     MetadataVersionMismatch,
-    /// Missing metadata (Zarr V2 only).
-    #[error("group metadata is missing (Zarr V2 only)")]
+    /// Missing metadata.
+    #[error("Metadata is missing")]
     MissingMetadata,
 }
 
@@ -119,18 +118,7 @@ impl Node {
         }
 
         // No metadata has been found
-        match version {
-            MetadataRetrieveVersion::Default | MetadataRetrieveVersion::V3 => {
-                // V3 supports missing metadata
-                Ok(NodeMetadata::Group(GroupMetadata::V3(
-                    GroupMetadataV3::default(),
-                )))
-            }
-            MetadataRetrieveVersion::V2 => {
-                // V2 does not support missing metadata
-                Err(NodeCreateError::MissingMetadata)
-            }
-        }
+        Err(NodeCreateError::MissingMetadata)
     }
 
     #[cfg(feature = "async")]
@@ -192,18 +180,7 @@ impl Node {
         }
 
         // No metadata has been found
-        match version {
-            MetadataRetrieveVersion::Default | MetadataRetrieveVersion::V3 => {
-                // V3 supports missing metadata
-                Ok(NodeMetadata::Group(GroupMetadata::V3(
-                    GroupMetadataV3::default(),
-                )))
-            }
-            MetadataRetrieveVersion::V2 => {
-                // V2 does not support missing metadata
-                Err(NodeCreateError::MissingMetadata)
-            }
-        }
+        Err(NodeCreateError::MissingMetadata)
     }
 
     #[deprecated(since = "0.15.0", note = "please use `open` instead")]
@@ -406,7 +383,7 @@ impl Node {
 mod tests {
     use crate::{
         array::{ArrayBuilder, ArrayMetadataOptions, FillValue},
-        group::GroupMetadata,
+        group::{GroupMetadata, GroupMetadataV3},
         storage::{store::MemoryStore, StoreKey, WritableStorageTraits},
     };
 
@@ -483,15 +460,12 @@ mod tests {
         serde_json::from_str::<NodeMetadata>(JSON_GROUP).unwrap();
     }
 
+    /// Implicit node support is removed since implicit groups were removed from the Zarr V3 spec
     #[test]
-    fn node_default() {
+    fn node_implicit() {
         let store = std::sync::Arc::new(MemoryStore::new());
         let node_path = "/node";
-        let node = Node::open(&store, node_path).unwrap();
-        assert_eq!(
-            node.metadata,
-            NodeMetadata::Group(GroupMetadata::V3(GroupMetadataV3::default()))
-        );
+        assert!(Node::open(&store, node_path).is_err());
     }
 
     #[test]
@@ -547,9 +521,13 @@ mod tests {
             )
             .unwrap();
         assert_eq!(
-            Node::open(&store, "/node").unwrap_err().to_string(),
+            Node::open(&store, "/node/array").unwrap_err().to_string(),
             "error parsing metadata for node/array/zarr.json: expected value at line 1 column 1"
         );
+        assert_eq!(
+            Node::open(&store, "/node").unwrap_err().to_string(),
+            "Metadata is missing"
+        );
     }
 
     #[test]