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

fix(consolidated metadata): skip .zmetadata key in members search #2356

Merged
merged 3 commits into from
Oct 14, 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
11 changes: 7 additions & 4 deletions src/zarr/core/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import itertools
import json
import logging
import warnings
from collections import defaultdict
from dataclasses import asdict, dataclass, field, fields, replace
from typing import TYPE_CHECKING, Literal, TypeVar, assert_never, cast, overload
Expand Down Expand Up @@ -1139,7 +1140,8 @@ async def _members(
raise ValueError(msg)
# would be nice to make these special keys accessible programmatically,
# and scoped to specific zarr versions
_skip_keys = ("zarr.json", ".zgroup", ".zattrs")
# especially true for `.zmetadata` which is configurable
_skip_keys = ("zarr.json", ".zgroup", ".zattrs", ".zmetadata")

# hmm lots of I/O and logic interleaved here.
# We *could* have an async gen over self.metadata.consolidated_metadata.metadata.keys()
Expand Down Expand Up @@ -1169,9 +1171,10 @@ async def _members(
# keyerror is raised when `key` names an object (in the object storage sense),
# as opposed to a prefix, in the store under the prefix associated with this group
# in which case `key` cannot be the name of a sub-array or sub-group.
logger.warning(
"Object at %s is not recognized as a component of a Zarr hierarchy.",
key,
warnings.warn(
f"Object at {key} is not recognized as a component of a Zarr hierarchy.",
UserWarning,
stacklevel=1,
)

def _members_consolidated(
Expand Down
31 changes: 25 additions & 6 deletions tests/v3/test_group.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import annotations

import contextlib
import pickle
import warnings
from typing import TYPE_CHECKING, Any, Literal, cast

import numpy as np
Expand Down Expand Up @@ -177,22 +179,33 @@ def test_group_members(store: Store, zarr_format: ZarrFormat, consolidated_metad
)
)

# this warning shows up when extra objects show up in the hierarchy
warn_context = pytest.warns(
UserWarning, match=r"Object at .* is not recognized as a component of a Zarr hierarchy."
)
if consolidated_metadata:
zarr.consolidate_metadata(store=store, zarr_format=zarr_format)
with warn_context:
zarr.consolidate_metadata(store=store, zarr_format=zarr_format)
# now that we've consolidated the store, we shouldn't get the warnings from the unrecognized objects anymore
# we use a nullcontext to handle these cases
warn_context = contextlib.nullcontext()
group = zarr.open_consolidated(store=store, zarr_format=zarr_format)

members_observed = group.members()
with warn_context:
members_observed = group.members()
# members are not guaranteed to be ordered, so sort before comparing
assert sorted(dict(members_observed)) == sorted(members_expected)

# partial
members_observed = group.members(max_depth=1)
with warn_context:
members_observed = group.members(max_depth=1)
members_expected["subgroup/subsubgroup"] = subsubgroup
# members are not guaranteed to be ordered, so sort before comparing
assert sorted(dict(members_observed)) == sorted(members_expected)

# total
members_observed = group.members(max_depth=None)
with warn_context:
members_observed = group.members(max_depth=None)
members_expected["subgroup/subsubgroup/subsubsubgroup"] = subsubsubgroup
# members are not guaranteed to be ordered, so sort before comparing
assert sorted(dict(members_observed)) == sorted(members_expected)
Expand Down Expand Up @@ -1091,8 +1104,8 @@ async def test_require_array(store: Store, zarr_format: ZarrFormat) -> None:


@pytest.mark.parametrize("consolidate", [True, False])
def test_members_name(store: Store, consolidate: bool):
group = Group.from_store(store=store)
async def test_members_name(store: Store, consolidate: bool, zarr_format: ZarrFormat):
group = Group.from_store(store=store, zarr_format=zarr_format)
a = group.create_group(name="a")
a.create_array("array", shape=(1,))
b = a.create_group(name="b")
Expand All @@ -1108,6 +1121,12 @@ def test_members_name(store: Store, consolidate: bool):
expected = ["/a", "/a/array", "/a/b", "/a/b/array"]
assert paths == expected

# regression test for https://github.com/zarr-developers/zarr-python/pull/2356
g = zarr.open_group(store, use_consolidated=False)
with warnings.catch_warnings():
warnings.simplefilter("error")
assert list(g)


async def test_open_mutable_mapping():
group = await zarr.api.asynchronous.open_group(store={}, mode="w")
Expand Down