Skip to content

fix: suggest store class when reading a packed zarr store by path - #2571

Open
JOhnsonKC201 wants to merge 2 commits into
scverse:mainfrom
JOhnsonKC201:fix/read-zarr-packed-store-hint
Open

fix: suggest store class when reading a packed zarr store by path#2571
JOhnsonKC201 wants to merge 2 commits into
scverse:mainfrom
JOhnsonKC201:fix/read-zarr-packed-store-hint

Conversation

@JOhnsonKC201

Copy link
Copy Markdown

Problem

zarr.open treats a path like adata.zarr.zip as a directory store, finds no
group in it, and raises a bare GroupNotFoundError. Nothing in that error hints
that the path has to be wrapped in a store class first, so the fix is hard to
guess:

adata.write_zarr(zarr.storage.ZipStore("adata.zarr.zip", mode="w"))

ad.io.read_zarr("adata.zarr.zip")
# zarr.errors.GroupNotFoundError: No group found in store
# StorePath(LocalStore, 'file://…/adata.zarr.zip') at path ''

Approach

This follows @flying-sheep's suggestion in
#2152 (comment). When
read_zarr gets a path-like whose suffix is known to hold a whole store in a
single file, the raised error gets a note naming the store class to use:

Did you mean `read_zarr(zarr.storage.ZipStore('adata.zarr.zip'))`?

Deliberately not done, per @ilan-gold's comment on the same issue: the
suffix is not used to open a ZipStore automatically. Reading such a path still
raises, so there is no magic-string store inference and no behaviour change
beyond the added note. add_note is already the codebase's idiom for enriching
I/O errors (anndata._io.utils.add_key_note).

The mapping from suffix to store class is a module-level constant, so other
single-file stores can be added later without touching the logic.

read_lazy opens user-supplied paths the same way
(anndata/experimental/backed/_io.py) and so has the same rough edge. I left it
alone to keep this focused on the reported issue. Happy to extend it there in
this PR or a follow-up, whichever you prefer.

Test plan

  • test_read_zarr_zip_path_suggests_store: asserts the note is attached,
    and that the incantation it suggests actually round-trips
  • test_read_zarr_dir_path_suggests_nothing: a directory store that holds
    no group gets no misleading suggestion
  • Both verified to fail before the source change and pass after
  • Full suite (-m "not dask_distributed"): 8051 passed, 1441 skipped,
    210 xfailed, 154 subtests passed in 7:45. The 3 failures are pre-existing
    and unrelated (all reproduce on main, and none touch this code path):
    test_write_large_categorical[zarr2/zarr3] is an OOM from running 16
    xdist workers locally and passes when run serially;
    test_concatenate_disk.py::test_output_dir_exists passes a Windows path
    to pytest.raises(match=...), which is an invalid regex (incomplete escape \U) on Windows only.
  • dask_distributed shard: 59 passed, 6 skipped
  • ruff check, ruff format --check, ruff check --preview --select=PLR0917,
    and codespell clean on the changed files

`zarr.open` treats a path like `adata.zarr.zip` as a directory store, so
`read_zarr` failed with a bare `GroupNotFoundError` that gave no hint that
the path needs wrapping in `zarr.storage.ZipStore`.

Attach a note pointing at the right store class when the path has a suffix
known to hold a whole store in one file. Reading such a path still fails, so
no magic-string store inference is introduced.
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.90%. Comparing base (f723aa8) to head (16e4573).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2571      +/-   ##
==========================================
- Coverage   87.78%   85.90%   -1.88%     
==========================================
  Files          49       49              
  Lines        7799     7816      +17     
==========================================
- Hits         6846     6714     -132     
- Misses        953     1102     +149     
Files with missing lines Coverage Δ
src/anndata/_io/zarr.py 84.61% <100.00%> (+2.61%) ⬆️

... and 9 files with indirect coverage changes

Writing an AnnData through a ZipStore makes write_zarr set the root
attributes after the group is written, so zarr.json is stored twice and
zipfile warns about the duplicate entry. That warning is an error under
--strict-warnings, which broke the zarr_io CI jobs.

Write a directory store and zip its contents instead, which is what a
user ends up with anyway. Also covers the non-path store branch, which
returns without adding a note.
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.

anndata.io.read_zarr incompatible with a Zarr ZipStorage

1 participant