Skip to content

docs: use in-memory stores for examples that don't demonstrate disk storage - #4289

Open
CAOShurong wants to merge 2 commits into
zarr-developers:mainfrom
CAOShurong:p3-3681-docs-memory-store
Open

docs: use in-memory stores for examples that don't demonstrate disk storage#4289
CAOShurong wants to merge 2 commits into
zarr-developers:mainfrom
CAOShurong:p3-3681-docs-memory-store

Conversation

@CAOShurong

Copy link
Copy Markdown

Description

Resolves #3681.

Documentation examples that are not specifically demonstrating local/remote
persistence were writing arrays to the local data/ directory at docs-build
time (the issue estimates ~20 arrays across quick-start and the user guide).
This PR:

  • Switches non-persistence demos to in-memory stores (memory:// URLs /
    MemoryStore() / empty-dict stores) so the build no longer litters
    artifacts on disk.
  • Converts the persistent-storage demos to clean up their own scratch
    directories instead of relying on a single global shutil.rmtree('data')
    at the top of quick-start.md (which also silently masked a
    ZipStore parent-dir bug — the zip demos now create data/ explicitly).
  • Updates quick-start.md prose that claimed the opening examples used a
    LocalStore (they now use in-memory stores).

storage.md, the LocalStore/ZipStore sections of quick-start.md, and the
create_hierarchy(store=LocalStore(root='data')) demo in groups.md are
intentionally left on-disk since they demonstrate file/disk backends.

Type of change

  • Documentation fix (broken docs, missing docs, accuracy improvements)

Checklist

  • I have read the CONTRIBUTING guide.
  • My changes follow the code style of this project.
  • I have added tests / the change is covered by existing tests that run in CI.
  • I have updated the changelog accordingly (added changes/3682.doc.md).

Verification

tests/test_docs.py executes every exec="true" / test="true" block:

61 passed, 2 skipped in ~25s

The 2 skips are pre-existing environment markers (cupy GPU example and a
docstring example that imports a nonexistent module) and are unrelated to
this change.

🤖 Generated with Claude Code

@d-v-b

d-v-b commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Code review

Found 4 issues:

  1. The cli.md migration example no longer opens the array it creates: each {} literal is a distinct dict, so zarr.create_array(store={}, ...) and zarr.open({}, zarr_format=3) get two unrelated in-memory stores. Since zarr.open defaults to mode 'a', the second call silently creates a new empty Group instead of opening the array, contradicting the surrounding prose. The old code worked because both calls reused the path "data/cli-demo.zarr". Fix: bind one store object (store = {}) and pass it to both calls, or use a named memory://cli-demo URL as elsewhere in this PR. (Not caught by tests/test_docs.py — the block has no result= assertion.)

# create a small array to open (stands in for the migrated store)
zarr.create_array(store={}, shape=(4, 4), chunks=(2, 2), dtype="i4")
zarr_with_v3_metadata = zarr.open({}, zarr_format=3)
```

  1. Stale prose in quick-start.md: the first example now writes to memory://quickstart-scale-demo, but the sentence below still says "This array was written to a LocalStore in the data/example-1.zarr directory" — now false, and it contradicts the new sentence added later on the same page ("While the examples above all used in-memory stores...").

`(10, 10)`, and filled it with random floating-point data. This array was
written to a `LocalStore` in the `data/example-1.zarr` directory.

  1. The performance.md pickle example should stay on disk: the prose above it deliberately contrasts persistent stores (only reopen parameters are pickled) with MemoryStore (data is copied into the pickle stream), and the example is labeled "pickle/unpickle a local store array". Switching it from store='data/perf-example-2.zarr' to store={} makes it demonstrate exactly the MemoryStore case the prose contrasts against — a fourth disk-intentional example alongside the ones this PR already exempts (storage.md, the quick-start ZipStore section, the groups.md create_hierarchy demo).

E.g., pickle/unpickle a local store array:
```python exec="true" session="performance" source="above" result="ansi"
import pickle
data = np.arange(100000)
z1 = zarr.create_array(store={}, shape=data.shape, chunks=data.shape, dtype=data.dtype)
z1[:] = data

  1. Wrong changelog filename: changes/README.md says the filename number is "the pull request number associated with this entry", and towncrier renders it as a link to pull/{issue}. The file is named 3682.doc.md, but BUG: Data corruption when writing partial shards with dask (concurrent writes to same shard) #3682 is an unrelated closed bug (partial-shard data corruption with dask), so the release notes would link to the wrong item. Should be 4289.doc.md (the PR resolves don't default to local storage in docs #3681, so the intended issue number is also off by one).

https://github.com/zarr-developers/zarr-python/blob/ccea1c4c85ec7b5705aab17cf0f895abcd6d4cb7/changes/3682.doc.md

Minor (below confidence threshold): the new cleanup before the groups.md create_hierarchy demo is shutil.rmtree('data', ignore_errors=True), which wipes the entire data/ directory (including quick-start's data/example-5.zip when pages run sequentially) rather than a scoped path like this PR's other cleanups.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

CAOShurong and others added 2 commits August 28, 2026 04:40
…torage (zarr-developers#3681)

Resolves zarr-developers#3681. Documentation examples that are
not specifically demonstrating local/remote persistence wrote arrays to the
local data/ directory at docs-build time (~20 arrays across quick-start and
the user guide). Switch non-persistence demos to in-memory stores
(memory:// URLs / MemoryStore / empty-dict stores) so the build no longer
litter artifacts on disk, and convert the persistent-storage demos to clean
up their own scratch directories instead of relying on a single global
rmtree at the top of quick-start.

Verified with tests/test_docs.py (61 passed, 2 skipped — cupy/s3 markers).

Co-Authored-By: Claude <noreply@anthropic.com>
@CAOShurong
CAOShurong force-pushed the p3-3681-docs-memory-store branch from ccea1c4 to b7c0821 Compare August 27, 2026 20:45
@CAOShurong

Copy link
Copy Markdown
Author

Follow-up fixes pushed in b7c0821e after the review:

  • the two CLI {} examples now reuse the same in-memory store;
  • quick-start text no longer refers to the removed LocalStore API;
  • the performance pickle example again uses a local disk-backed store as described;
  • the changelog fragment is now changes/4289.doc.md.

Verified with the relevant documentation examples (6 passed), mkdocs build --strict, ci/lint_docs.py, towncrier check, and git diff --check. Ready for re-review.

@CAOShurong

Copy link
Copy Markdown
Author

The single failing required-matrix job is unrelated to this docs-only diff: tests/test_fused_pipeline.py::test_async_chunk_transform_matches_sync[bb] compares gzip bytes whose only difference is the one-second mtime in the gzip header (...bc¢�j... vs ...bb¢�j...). The PR changes no fused-pipeline or gzip code. I reran that exact test five times locally on head b7c0821e; all 5 passed. The remaining cancellations are the matrix's fail-fast consequence; docs, lint, wheel, benchmark, and other test contexts are green.

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.

don't default to local storage in docs

2 participants