Run doctests via pytest and fix broken doctests.#4000
Conversation
github-actions
Bot
added
the
needs release notes
github-actions
Bot
removed
the
needs release notes
| >>> arr = zarr.create_array(store, shape=(100, 80), chunks=(30, 40)) | ||
| >>> import zarr.storage | ||
| >>> store = zarr.storage.MemoryStore() | ||
| >>> arr = zarr.create_array(store, dtype="i1", shape=(100, 80), chunks=(30, 40)) |
There was a problem hiding this comment.
for these kinds of examples, we can use the URL form of the memorystore:
arr = zarr.create_array("memory:///", dtype="i1", shape=(100, 80), chunks=(30, 40))
There was a problem hiding this comment.
Oh, nice! I'll tweak.
There was a problem hiding this comment.
out of scope ofc but we should labor to comply with the URL rules defined here: https://en.wikipedia.org/wiki/File_URI_scheme, specifically as regards number of slashes
There was a problem hiding this comment.
you can also pass an empty dict {} as the store, that's probably even shorter, but it means you can't refer to the store later
There was a problem hiding this comment.
Sorry, not at this spot, but elsewhere. Let me get more specifics for you.
There was a problem hiding this comment.
that reveals that the docstring tests are running in a shared session, which we probably don't want
There was a problem hiding this comment.
Oh, I think I see what's happening. It succeeds on the first test it hits that uses "memory:///", but all subsequent tests that use "memory:///" produce that error. Seems like they all use the same instance. Caching?
There was a problem hiding this comment.
i wouldn't call it caching, they just use the same in-memory store. But I don't think separate docstring snippets should run in the same sessions -- they should be run in completely separate sessions. is that something we can control?
There was a problem hiding this comment.
In this case, I don't think we get the isolation you expect. If using "memory:///" causes some global state creation, it will be seen across all tests. Only tests running in separate interpreters would be fully isolated.
If you really would prefer use of the string, I propose opening a separate issue, as I don't think it's worth blocking this PR, especially since these are all pre-existing doctests, adjusted in this PR only to resolve pre-existing problems with them.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4000 +/- ##
==========================================
+ Coverage 93.49% 93.55% +0.05%
==========================================
Files 88 88
Lines 11873 11873
==========================================
+ Hits 11101 11108 +7
+ Misses 772 765 -7
🚀 New features to boost your workflow:
|
2 participants
Runs doctest via pytest and fixes all broken doctests.
Fixes #3498
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 indocs/user-guide/*.mdchanges/