From ebe267096e146ef14f94d25fb14140678fe6dc68 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:03:04 -0400 Subject: [PATCH 1/6] Fix incorrect Group.nmembers for consolidated metadata --- src/zarr/core/group.py | 2 +- tests/test_group.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index a868ee31fa..d722b9a667 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -1307,7 +1307,7 @@ async def nmembers( # check if we can use consolidated metadata, which requires that we have non-None # consolidated metadata at all points in the hierarchy. if self.metadata.consolidated_metadata is not None: - return len(self.metadata.consolidated_metadata.flattened_metadata) + return len([x for x in self.metadata.consolidated_metadata.flattened_metadata if x.count("/") <= max_depth]) # TODO: consider using aioitertools.builtins.sum for this # return await aioitertools.builtins.sum((1 async for _ in self.members()), start=0) n = 0 diff --git a/tests/test_group.py b/tests/test_group.py index ac1afb539b..9ca32532bb 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -1118,6 +1118,13 @@ async def test_group_members_async(store: Store, consolidated_metadata: bool) -> "consolidated_metadata", None, ) + # test depth=0 + nmembers = await group.nmembers(max_depth=0) + assert nmembers == 2 + # test depth=1 + nmembers = await group.nmembers(max_depth=1) + assert nmembers == 4 + # test depth=None all_children = sorted( [x async for x in group.members(max_depth=None)], key=operator.itemgetter(0) ) From 32e3de6d710d5d8f97b4c411dbe2988f11a2dfee Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:13:41 -0400 Subject: [PATCH 2/6] Handle max_depth==None --- src/zarr/core/group.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index d722b9a667..b05d690a03 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -1307,7 +1307,13 @@ async def nmembers( # check if we can use consolidated metadata, which requires that we have non-None # consolidated metadata at all points in the hierarchy. if self.metadata.consolidated_metadata is not None: - return len([x for x in self.metadata.consolidated_metadata.flattened_metadata if x.count("/") <= max_depth]) + if max_depth is not None and max_depth < 0: + raise ValueError(f"max_depth must be None or >= 0. Got '{max_depth}' instead") + if max_depth is None: + return len(self.metadata.consolidated_metadata.flattened_metadata) + else: + return len([x for x in self.metadata.consolidated_metadata.flattened_metadata if x.count("/") <= max_depth]) + # TODO: consider using aioitertools.builtins.sum for this # return await aioitertools.builtins.sum((1 async for _ in self.members()), start=0) n = 0 From 982756a43877d15156b0acadb9315a6cebc73ccc Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:17:21 -0400 Subject: [PATCH 3/6] Remove extra whitespace --- src/zarr/core/group.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index b05d690a03..176a5baf5e 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -1313,7 +1313,6 @@ async def nmembers( return len(self.metadata.consolidated_metadata.flattened_metadata) else: return len([x for x in self.metadata.consolidated_metadata.flattened_metadata if x.count("/") <= max_depth]) - # TODO: consider using aioitertools.builtins.sum for this # return await aioitertools.builtins.sum((1 async for _ in self.members()), start=0) n = 0 From 7d02b6d9210781a45f18441ae3442c02abf2fa63 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:19:44 -0400 Subject: [PATCH 4/6] ruff format --- src/zarr/core/group.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index 176a5baf5e..a398aa01aa 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -1312,7 +1312,13 @@ async def nmembers( if max_depth is None: return len(self.metadata.consolidated_metadata.flattened_metadata) else: - return len([x for x in self.metadata.consolidated_metadata.flattened_metadata if x.count("/") <= max_depth]) + return len( + [ + x + for x in self.metadata.consolidated_metadata.flattened_metadata + if x.count("/") <= max_depth + ] + ) # TODO: consider using aioitertools.builtins.sum for this # return await aioitertools.builtins.sum((1 async for _ in self.members()), start=0) n = 0 From c42b68117780247fde5203837916dc7411e4b68f Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:41:02 -0400 Subject: [PATCH 5/6] Test for negative depth --- tests/test_group.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_group.py b/tests/test_group.py index 9ca32532bb..7705fa205a 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -1131,6 +1131,9 @@ async def test_group_members_async(store: Store, consolidated_metadata: bool) -> assert len(all_children) == 4 nmembers = await group.nmembers(max_depth=None) assert nmembers == 4 + # test depth<0 + with pytest.raises(ValueError, match="max_depth"): + await group.nmembers(max_depth=-1) async def test_require_group(store: LocalStore | MemoryStore, zarr_format: ZarrFormat) -> None: From 9df9683f8f8fe99411045089049194dcd678dfa3 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Wed, 23 Jul 2025 14:43:44 -0400 Subject: [PATCH 6/6] Document changes --- changes/3287.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/3287.bugfix.rst diff --git a/changes/3287.bugfix.rst b/changes/3287.bugfix.rst new file mode 100644 index 0000000000..a4eaa35312 --- /dev/null +++ b/changes/3287.bugfix.rst @@ -0,0 +1 @@ +Fixes Group.nmembers() ignoring depth when using consolidated metadata.