Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove implicit group support #51

Merged
merged 1 commit into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 13 additions & 36 deletions src/group.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -207,20 +211,7 @@
}

// 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)
}
}

Expand Down Expand Up @@ -276,20 +267,7 @@
}

// 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)

Check warning on line 270 in src/group.rs

View check run for this annotation

Codecov / codecov/patch

src/group.rs#L270

Added line #L270 was not covered by tests
}
}

Expand All @@ -305,8 +283,8 @@
/// 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,
}

Expand Down Expand Up @@ -603,12 +581,11 @@
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());
}
}
48 changes: 13 additions & 35 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

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,
Expand Down Expand Up @@ -59,8 +58,8 @@
/// 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,
}

Expand Down Expand Up @@ -119,18 +118,7 @@
}

// 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")]
Expand Down Expand Up @@ -192,18 +180,7 @@
}

// 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)

Check warning on line 183 in src/node.rs

View check run for this annotation

Codecov / codecov/patch

src/node.rs#L183

Added line #L183 was not covered by tests
}

#[deprecated(since = "0.15.0", note = "please use `open` instead")]
Expand Down Expand Up @@ -406,7 +383,7 @@
mod tests {
use crate::{
array::{ArrayBuilder, ArrayMetadataOptions, FillValue},
group::GroupMetadata,
group::{GroupMetadata, GroupMetadataV3},
storage::{store::MemoryStore, StoreKey, WritableStorageTraits},
};

Expand Down Expand Up @@ -483,15 +460,12 @@
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]
Expand Down Expand Up @@ -547,9 +521,13 @@
)
.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]
Expand Down