Skip to content

feat: add type-safe get_array and get_group methods to AsyncGroup and Group#4128

Open
d-v-b wants to merge 19 commits into
zarr-developers:mainfrom
d-v-b:claude/great-blackburn-02f41c
Open

feat: add type-safe get_array and get_group methods to AsyncGroup and Group#4128
d-v-b wants to merge 19 commits into
zarr-developers:mainfrom
d-v-b:claude/great-blackburn-02f41c

Conversation

@d-v-b

@d-v-b d-v-b commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Summary

Adds type-safe accessors for group members: AsyncGroup.get_array / AsyncGroup.get_group and their sync mirrors Group.get_array / Group.get_group. Each returns the child node at the given path or raises — ArrayNotFoundError / GroupNotFoundError when no node exists, and ContainsGroupError / ContainsArrayError when a node exists but is the wrong kind.

Today group[key] returns the union Array | Group, so callers who know a key refers to an array still need an isinstance check or a cast to get a usefully-typed value. These methods do that narrowing once, with a clear error contract, using the same exception vocabulary as open_array / open_group.

Design notes:

  • Path semantics mirror getitem exactly: nested paths like "subgroup/subarray" are supported, on both the plain and consolidated-metadata lookup routes.
  • The methods are thin wrappers over getitem, so there is no new I/O logic.
  • Several existing tests that fetched a member and then manually narrowed the type (walrus + isinstance asserts in test_zip.py, getitem + isinstance in the consolidated-metadata tests) now use the new methods.

For reviewers

  • The main API-design choices worth a second look: the get_array / get_group names (vs. alternatives like an expect parameter on getitem), accepting nested paths rather than restricting to direct children, and the choice of exception types for the two failure modes.
  • Confident in: behavior of the wrappers themselves (they delegate to getitem), test coverage (happy path with nested access plus one test per failure mode, across store types and both zarr formats).

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

(Left unchecked: opened by an agent on @d-v-b's behalf; Davis will check this after his own review.)

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

🤖 Generated with Claude Code

dependabot Bot and others added 15 commits May 31, 2026 19:28
…#176)

Bumps the actions group with 8 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [prefix-dev/setup-pixi](https://github.com/prefix-dev/setup-pixi) | `0.9.5` | `0.9.6` |
| [codecov/codecov-action](https://github.com/codecov/codecov-action) | `6.0.0` | `6.0.1` |
| [github/issue-metrics](https://github.com/github/issue-metrics) | `4.2.2` | `4.2.7` |
| [j178/prek-action](https://github.com/j178/prek-action) | `2.0.3` | `2.0.4` |
| [actions/upload-artifact](https://github.com/actions/upload-artifact) | `7.0.0` | `7.0.1` |
| [actions/download-artifact](https://github.com/actions/download-artifact) | `7.0.0` | `8.0.1` |
| [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) | `1.13.0` | `1.14.0` |
| [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action) | `0.5.3` | `0.5.6` |



Updates `prefix-dev/setup-pixi` from 0.9.5 to 0.9.6
- [Release notes](https://github.com/prefix-dev/setup-pixi/releases)
- [Commits](prefix-dev/setup-pixi@1b2de7f...5185adf)

Updates `codecov/codecov-action` from 6.0.0 to 6.0.1
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@57e3a13...e79a696)

Updates `github/issue-metrics` from 4.2.2 to 4.2.7
- [Release notes](https://github.com/github/issue-metrics/releases)
- [Commits](github-community-projects/issue-metrics@c9e9838...1e38d5e)

Updates `j178/prek-action` from 2.0.3 to 2.0.4
- [Release notes](https://github.com/j178/prek-action/releases)
- [Commits](j178/prek-action@6ad8027...bdca6f1)

Updates `actions/upload-artifact` from 7.0.0 to 7.0.1
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v7...043fb46)

Updates `actions/download-artifact` from 7.0.0 to 8.0.1
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v7...3e5f45b)

Updates `pypa/gh-action-pypi-publish` from 1.13.0 to 1.14.0
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.13.0...cef2210)

Updates `zizmorcore/zizmor-action` from 0.5.3 to 0.5.6
- [Release notes](https://github.com/zizmorcore/zizmor-action/releases)
- [Commits](zizmorcore/zizmor-action@b1d7e1f...5f14fd0)

---
updated-dependencies:
- dependency-name: prefix-dev/setup-pixi
  dependency-version: 0.9.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
- dependency-name: codecov/codecov-action
  dependency-version: 6.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
- dependency-name: github/issue-metrics
  dependency-version: 4.2.7
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
- dependency-name: j178/prek-action
  dependency-version: 2.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
- dependency-name: actions/upload-artifact
  dependency-version: 7.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
- dependency-name: actions/download-artifact
  dependency-version: 8.0.1
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: pypa/gh-action-pypi-publish
  dependency-version: 1.14.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: actions
- dependency-name: zizmorcore/zizmor-action
  dependency-version: 0.5.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… Group

Add `get_array` and `get_group` methods that return the child node at
a given path or raise: `ArrayNotFoundError`/`GroupNotFoundError` when
no node exists, and `ContainsGroupError`/`ContainsArrayError` when the
node is the wrong kind. Paths mirror `getitem` semantics, so nested
paths like "subgroup/subarray" work on both the plain and
consolidated-metadata lookup routes.

Existing tests that fetched a child via `getitem` and then manually
narrowed the type with `isinstance` asserts or a walrus expression now
use the new methods instead.

Assisted-by: ClaudeCode:claude-fable-5
…et_group

The multi-argument template form of BaseZarrError.__init__ is
documented as deprecated; build the message string at the raise site
instead, matching every other call site in the codebase.

Assisted-by: ClaudeCode:claude-fable-5
@github-actions github-actions Bot added the needs release notes Automatically applied to PRs which haven't added release notes label Jul 7, 2026
Assisted-by: ClaudeCode:claude-fable-5
@github-actions github-actions Bot removed the needs release notes Automatically applied to PRs which haven't added release notes label Jul 7, 2026
@d-v-b

d-v-b commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

cc @lhoupert

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.63%. Comparing base (6876120) to head (44e37a1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4128      +/-   ##
==========================================
+ Coverage   93.62%   93.63%   +0.01%     
==========================================
  Files          90       90              
  Lines       11936    11962      +26     
==========================================
+ Hits        11175    11201      +26     
  Misses        761      761              
Files with missing lines Coverage Δ
src/zarr/core/group.py 95.33% <100.00%> (+0.13%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-v-b d-v-b requested a review from maxrjones July 7, 2026 20:53
@maxrjones

Copy link
Copy Markdown
Member

I think the main design question is actually whether to have this as two functions or one with an expected class argument (analogous to the model https://www.pytables.org/usersguide/libref/file_class.html#tables.File.get_node). Did you already consider that alternative design @d-v-b?

@d-v-b

d-v-b commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

I didn't really consider that design because we have a closed set of nodes with just 2 options: array or group. I think if we had a situation where the set of possible node types was dynamic or unbounded (which might be what is going on for pytables?) then get_node might make sense? But even then, I don't think that pytables API is a good idea, compared to giving users tools sufficient to do their own type checking. But since we have just 2 options, and we already have create_array and create_group, get_array and get_group seemed the most ergonomic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants