Skip to content

Conversation

d-v-b
Copy link
Contributor

@d-v-b d-v-b commented Oct 2, 2025

Adds doctests. This PR builds atop #3366, and so should not be merged until #3366 is in.

I add a new test file in test/test_docs.py that collects executable examples and runs them, ensuring that no errors are raised.

The goal here is to go from 0 doctests to some doctests. But we need to ultimately do more than "some doctests". We need tests that compare the output of the executed code against what we expected. I'm happy to take that as a contribution / addition to this PR if it's simple, or we can do that in a subsequent PR.

Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.86%. Comparing base (e0751ab) to head (a897230).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3500   +/-   ##
=======================================
  Coverage   61.86%   61.86%           
=======================================
  Files          85       85           
  Lines       10111    10111           
=======================================
  Hits         6255     6255           
  Misses       3856     3856           
Files with missing lines Coverage Δ
src/zarr/api/synchronous.py 36.61% <ø> (ø)
src/zarr/codecs/numcodecs/_codecs.py 38.46% <ø> (ø)
src/zarr/core/array.py 68.62% <ø> (ø)
src/zarr/core/attributes.py 46.15% <ø> (ø)
src/zarr/core/dtype/__init__.py 30.00% <ø> (ø)
src/zarr/core/dtype/npy/string.py 44.44% <ø> (ø)
src/zarr/core/dtype/npy/structured.py 56.38% <ø> (ø)
src/zarr/core/group.py 70.24% <ø> (ø)
src/zarr/core/indexing.py 69.46% <ø> (ø)
src/zarr/core/sync.py 61.68% <ø> (ø)
... and 5 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-v-b
Copy link
Contributor Author

d-v-b commented Oct 2, 2025

closes #3498

@d-v-b d-v-b requested a review from a team October 2, 2025 12:16
Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-v-b do you have a vision for how to match the executed code output to the expected results? Currently, I think this PR is a more complicated version of mkdocs build --strict so without a clear path to output comparisons I don't find it super compelling

@@ -0,0 +1 @@
Adds `zarr.experimental.cache_store.CacheStore`, a `Store` that implements caching by combining two other `Store` instances. See the [docs page](https://zarr.readthedocs.io/en/latest/user-guide/cache-store) for more information about this feature. No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-v-b can you please remove this addition? It is already properly documented using the PR number in https://github.com/zarr-developers/zarr-python/blob/main/changes/3366.feature.md

@d-v-b
Copy link
Contributor Author

d-v-b commented Oct 2, 2025

@d-v-b do you have a vision for how to match the executed code output to the expected results? Currently, I think this PR is a more complicated version of mkdocs build --strict so without a clear path to output comparisons I don't find it super compelling

mkdocs build --strict requires mkdocs and is not part of our test suite (i.e., something that you can run via pytest). This PR adds actual tests, which is the direction we need to go. Unlike checking for build failures, we can extend the approach here to compare output against expected output pretty easily.

But you reminded me that this PR can be made much simpler with pytest-examples. I'll push some changes that simplify things.

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Oct 3, 2025
@d-v-b
Copy link
Contributor Author

d-v-b commented Oct 3, 2025

@d-v-b do you have a vision for how to match the executed code output to the expected results? Currently, I think this PR is a more complicated version of mkdocs build --strict so without a clear path to output comparisons I don't find it super compelling

mkdocs build --strict requires mkdocs and is not part of our test suite (i.e., something that you can run via pytest). This PR adds actual tests, which is the direction we need to go. Unlike checking for build failures, we can extend the approach here to compare output against expected output pretty easily.

But you reminded me that this PR can be made much simpler with pytest-examples. I'll push some changes that simplify things.

It was not as simple as I thought -- the fact that our docs uses code blocks organized into sessions requires the test machinery in this PR. Since the examples in our docs are not currently set up for output checking, I left those tests as-is, but included a commented code block that, once commented, will check example code output.

I also added a test for examples defined in our docstrings. We have some docstring examples that either require session logic (like the examples in our docs) or require a re-write, so I largely left those alone. I did fix #3503 though, and many other broken docstring examples.

Comment on lines +109 to +110
# result = eval_example.run_print_check(example, module_globals=module_globals)
result = eval_example.run(example, module_globals=module_globals)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxrjones switching eval_example.run to eval_example.run_print_check will turn on output checks for the docs.

async_arr = await AsyncArray.open(store)
return async_arr
async_arr = asyncio.run(example())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our async examples previously used top-level await, which is not generally available in python interpreters. We now use asyncio.run()

@d-v-b d-v-b merged commit 49db79a into zarr-developers:main Oct 3, 2025
31 checks passed
@d-v-b d-v-b mentioned this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs release notes Automatically applied to PRs which haven't added release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants