-
-
Notifications
You must be signed in to change notification settings - Fork 309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use doctests for guide #2623
Use doctests for guide #2623
Conversation
Thanks for doing this @dstansby - how can we run the doc tests? Are they run automatically when you build the docs or in CI? |
They can be turned on to run with pytest in the normal test runs. Do you think setting that up should block v3? |
doctests are being run on v2 (see #2552 where I had to fix some recently), so there should be a pattern to copy there to enable them for v3. |
okay, I think we should do that here before merging. The main reason I went with Ipython was to guarantee that the docs were reflecting exactly what Zarr produces. So we don't want to loose that. |
c95f42b
to
219df84
Compare
I had a quick try, but it's not trivial (but probably not complicated either) to enable doctests. To keep things simple here I'd advocate enabling doctests as out of scope for this PR, and we can do it in a follow up to all the other PRs. FWIW I have run all the doctests locally to generate the output, so it should be correct at the moment. |
219df84
to
1396976
Compare
@dstansby - can you describe the issues with getting the doc tests passing here? Since we're reverting work that ensures the doc can be run, I'd like to, at a minimum, understand why this route isn't working. |
I don't think this is stopping the doc build from working? The readthedocs failures in this PR are the same as in the branch it's based on (ie #2589) I'm happy to either enable doctests in this PR, or in a follow up PR to #2589 as you prefer. I thought the preference was to merge #2589 ASAP, so I didn't want to hold up this (and therefore #2589) by working out the doctests, but I can do that (but maybe not today) if you think that should block #2589? |
1396976
to
ace1c13
Compare
Let's enable doctests in this PR. Otherwise, we're going to immediately break things in the docs and not know it. |
👍 doctests enabled in this PR now |
* docs: split tutorial into multiple user guide sections * Apply suggestions from code review Co-authored-by: David Stansby <dstansby@gmail.com> * respond to david's review * add todos for remainig api changes * docs: add new top level about page (#2592) * docs: add new top level about page * fixup * fixup * fixup * docs: add docs on extending zarr 3 (#2597) * docs: add docs on extending zarr 3 * Apply suggestions from code review Co-authored-by: David Stansby <dstansby@gmail.com> * move note up * remove test.py (#2612) * Note that whole directories can be deleted in LocalStore (#2606) * fix: run-coverage command now tracks src directory (#2615) * fix doc build * Update docs/user-guide/extending.rst --------- Co-authored-by: Norman Rzepka <code@normanrz.com> Co-authored-by: David Stansby <dstansby@gmail.com> Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com> * Use doctests for guide (#2623) * Use doctests for arrays.rst * Use doctests for attributes.rst * Use doctests for config.rst * Use doctests for consolidated metadata * Use doctests for groups.rst * Use doctests for preformance.rst * Use doctests for storage.rst * Remove ipython config for docs * Fix performance doctest output * Enable doctests * Add a doctest CI run * Remove rmtrees * Delete data dir before doctests * Fix doctests * fix doctests for arrays.rst * fix doctests for consolidated_metadata.rst * fixes for doctest * tests * debugging * debugging * debugging * debugging * debugging --------- Co-authored-by: David Stansby <dstansby@gmail.com> Co-authored-by: Norman Rzepka <code@normanrz.com> Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com> Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
This reverts using IPython for code examples in #2589, and is targeted to that branch (not main). See #2589 (review) for the motivation for not switching to IPython, but sticking with the doctest status-quo