docs: split tutorial into multiple user guide sections #2589

jhamman · 2024-12-29T21:44:19Z

This PR updates Zarr's documentation to include a new multi-page user-guide, in-place of the prior single page tutorial.

Notes for reviewers:

This was split off from Doc updates for 3.0 release #2568
Most content remains unchanged from the tutorial, despite the large diff line count. Some new sections were added in 2568. The only net new section added was config.rst.
Examples now use the Ipython sphinx directive

closes #1767

dstansby

Liking the new sections, makes it much easier to read than the old tutorial 👍

A couple of major questions:

The "copying/migrating data" tutorial section wasn't migrated from the old tutorial, is this intentional?

For IPython, what was the motivation for switching to this from doctests? As it stands it has a few major drawbacks:

Trying to copy/paste a block of code doesn't work, because it selects the Out[...] text too:
The expected output isn't embedded any more. This gave us another layer of regression tests when we ran the tutorial as a doc test, and on top of making sure the code runs, made sure the output was as expected.
I find having a line break between each line of code makes it much less readable - old vs new:

Is it possible to fix these with new IPython plugin?

Finally, what's the plan for all the TODOs (especially the numcodecs and Sharding bits, and configuring blosc)? To do them in another PR before 3.0? If so would be good to open issues for these so they're not lost and released as TODOs in 3.0

docs/index.rst

docs/user-guide/arrays.rst

docs/user-guide/performance.rst

pyproject.toml

docs/user-guide/index.rst

docs/user-guide/config.rst

Co-authored-by: David Stansby <[email protected]>

jhamman · 2025-01-01T14:54:33Z

Thanks for the review @dstansby!

Liking the new sections, makes it much easier to read than the old tutorial 👍

A couple of major questions:

The "copying/migrating data" tutorial section wasn't migrated from the old tutorial, is this intentional?

We have not implemented copy, copy_all, and copy_store yet. I'll add these to the list of TODOs in the migration guide if they are not there already.

For IPython, what was the motivation for switching to this from doctests?

For one, we weren't using the doc tests! Beyond that, this isn't something I feel super strong about. We've had a lot of success using the Ipython directive in the Xarray project.

As it stands it has a few major drawbacks:

Trying to copy/paste a block of code doesn't work, because it selects the Out[...] text too:

Not true if you use the copy button, example below:

z1[:] = 42

z1[0, :] = np.arange(10000)

z1[:, 0] = np.arange(10000)

I think there is probably a way to further configure the style.

The expected output isn't embedded any more. This gave us another layer of regression tests when we ran the tutorial as a doc test, and on top of making sure the code runs, made sure the output was as expected.

We can insert assertions in hidden cells as desired. This would achieve the same thing.

I find having a line break between each line of code makes it much less readable - old vs new:

Is it possible to fix these with new IPython plugin?

Likely yes. Can we iterate on the styling after this first PR goes in?

Finally, what's the plan for all the TODOs (especially the numcodecs and Sharding bits, and configuring blosc)? To do them in another PR before 3.0? If so would be good to open issues for these so they're not lost and released as TODOs in 3.0

I'm waiting #2463. I have a branch combining these but I'll need to start it over now that I've split up the docs refactor. So yes, let's do them in another PR.

…into docs/3.0-user-guide

dstansby · 2025-01-01T22:18:17Z

Trying to copy/paste a block of code doesn't work, because it selects the Out[...] text too:

Not true if you use the copy button

This isn't a solution if you want to select two lines of code from a longer block.

This gave us another layer of regression tests when we ran the tutorial as a doc test, and on top of making sure the code runs, made sure the output was as expected.

We can insert assertions in hidden cells as desired. This would achieve the same thing.

But with a much higher maintenance burden I think? We would have to manually concoct a hidden assertion for every code block in this case, whereas with the doctests we can just generate all the output automatically.

So given there don't seem to be any benefits to using the IPython plugin, but there still seem to be drawbacks, I think we should keep existing code block style for now.

re. the TODOs, I'm a bit tired to do it now but tomorrow I can open issues to keep track of them (if you haven't already).

jhamman · 2025-01-01T22:42:44Z

My view is that the most important thing is to have documentation that actually runs! I'm happy to hand this off to someone with sufficient experience with doctests (maybe @dstansby) but that isn't me. If no one is volunteering to do it right now, I'm going to press forward with this solution and we can revisit when someone else has time/energy for another doctest effort. I'll also note that these aren't mutually exclusive concepts (reading up on the ipython extension is probably worthwhile).

dstansby · 2025-01-02T11:13:31Z

In order to keep this moving, lets drop the switch to using the IPython plugin, since that shouldn't block a v3 release.

dstansby

Marking as request changes to revert the IPython change. @jhamman I'd be happy to change the code snippets in code back, but don't want to push to your PR without permission - let me know either way.

jhamman · 2025-01-02T14:28:12Z

If you can update the code snippets today, feel free to ~~push to this branch~~ open a PR (this branch is in the zarr-python repo). I'm all checked in. Like I said above, I'm not planning on reverting the change myself though.

* docs: add new top level about page * fixup * fixup * fixup

dstansby · 2025-01-02T15:07:46Z

👍 I can take a look. I think it's a reasonable ask to revert the change though, and not merge before it's reverted.

* docs: add docs on extending zarr 3 * Apply suggestions from code review Co-authored-by: David Stansby <[email protected]> * 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 <[email protected]> Co-authored-by: David Stansby <[email protected]> Co-authored-by: Davis Bennett <[email protected]>

normanrz · 2025-01-02T18:29:09Z

I think it's a reasonable ask to revert the change though, and not merge before it's reverted.

To be honest, I think it is more important to merge this PR quickly because of the dependent PRs. I feel this is blocking our progress right now. We can change away from IPython at any later point in time.

d-v-b · 2025-01-02T18:38:27Z

I think it's a reasonable ask to revert the change though, and not merge before it's reverted.

To be honest, I think it is more important to merge this PR quickly because of the dependent PRs. I feel this is blocking our progress right now. We can change away from IPython at any later point in time.

+1 to this, lets get this PR in and sort out doctest / ipython later

dcherian · 2025-01-02T18:40:00Z

I concur. Working doc examples are very valuable!

dstansby · 2025-01-02T18:57:26Z

I'm sorry, but I really want to insist we don't merge this with IPython in it, because it's a change that has demonstratable downsides. On a point of principle, just opening a large PR shouldn't come with an assumption that it's going to get merged without engaging with review. On a practical point, I spent about an hour sorting this out at #2623 today. So 🙏 can folks review and merge #2623 into this PR first.

d-v-b · 2025-01-02T20:15:43Z

the status quo which is doctests, to avoid holding up v3.

admittedly I'm late to this discussion, but my impression was that the status quo was that the doctests were not actually getting run, resulting in stale example code in the docs. I don't think anyone wants that.

So the next question is, how can we ensure that the docs contain real examples? Either we make the doctests work, or we use another strategy, e.g. running code when we build the docs (the ipython directive). Given the current context, we should do whichever of these two is most expedient, in the interest of moving things forward -- the most important thing is that the generated docs are correct. This allows everyone to hack on the docs productively. IMO, the only thing that should block that part of this PR is a functionally equivalent solution to the problem solved with the ipython directive. Does that make sense?

dstansby · 2025-01-02T20:16:46Z

admittedly I'm late to this discussion, but my impression was that the status quo was that the doctests were not actually getting run, resulting in stale example code in the docs. I don't think anyone wants that.

👍 , which is why I fixed the output in #2623...

jhamman · 2025-01-02T20:20:20Z

Let's deescalate this a bit.

If we can't get the doctests pr passing with ci today, we can merge this as is (with ipython) and go back to doctests as soon as it's ready.

I'm hearing general agreement that this PR is directionally aligned with where we want to go. Let's continue to iterate but in a non blocking way.

@dstansby - I know this isn't your ideal but I think it's what is best for the project overall.

dstansby · 2025-01-02T20:30:45Z

Okay, quick update, I'm trialling enabling doctests at #2626, which I think I have working. If it works, that PR should fail, and then I'll close and merge into #2623. Sorry it's a bit complex, but avoids me messing around with GH actions config in #2623 to get the tests running.

* 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

Changes addressed

dstansby · 2025-01-02T23:22:46Z

Quick update, I've:

added a new doctests CI run (was much simpler than running doctests in all the existing test runs)
added a new hatch environment that can be used to run the doctests (hatch run doctest:run) or automatically fix the output if it changes (hatch run doctest:fix)

Some of the doctests need fixing because of #2463 being merged, but hopefully the new CI job and hatch env make that a lot easier for someone else to do.

dstansby

Doctests need fixing - otherwise I think this is 👍

normanrz · 2025-01-03T10:51:01Z

Doctests need fixing - otherwise I think this is 👍

Working on it.

dstansby

LGTM 🚢 - a couple of optional comments. I haven't reviewed any of the actual content this time.

docs/user-guide/consolidated_metadata.rst

dstansby · 2025-01-03T14:30:04Z

tests/test_tree.py

@@ -10,6 +11,8 @@

 @pytest.mark.parametrize("root_name", [None, "root"])
 def test_tree(root_name: Any) -> None:
+    os.environ["OVERRIDE_COLOR_SYSTEM"] = "truecolor"


Just out of interest, why's this needed?

test_tree.py expects the tree to be rendered with the control sequences for text formatting (e.g. bold font). However, by default rich doesn't include that when running within pytest, because it detects that it is not running in a real terminal as the output is captured by pytest. This overrides the behavior in rich. This issue only surfaced now, because I added rich to the test dependencies and therefore test_tree.py wasn't skipped anymore.
https://github.com/zarr-developers/zarr-python/blob/main/tests/test_tree.py#L31-L54

docs: split tutorial into multiple user guide sections

01bc352

This was referenced Dec 29, 2024

docs: add new top level about page #2592

Merged

docs: replace getting_started page with quickstart #2590

Merged

docs: consolidate developer docs + update contributing page for v3 #2593

Merged

jhamman added the documentation Improvements to the documentation label Dec 29, 2024

This was referenced Dec 29, 2024

docs: add docs on using asyncio with zarr #2595

Open

docs: add migration page to user guide #2596

Merged

docs: add docs on extending zarr 3 #2597

Merged

jhamman added this to the 3.0.0 milestone Dec 30, 2024

jhamman mentioned this pull request Dec 30, 2024

doc: tutorial updates for 3.0 #2401

Closed

6 tasks

dstansby reviewed Dec 30, 2024

View reviewed changes

Apply suggestions from code review

1001994

Co-authored-by: David Stansby <[email protected]>

jhamman added 2 commits January 1, 2025 09:17

respond to david's review

d88f0c2

Merge branch 'main' of https://github.com/zarr-developers/zarr-python …

e434502

…into docs/3.0-user-guide

add todos for remainig api changes

eb8d9bf

dstansby previously requested changes Jan 2, 2025

View reviewed changes

docs: add new top level about page (#2592)

7c163e8

* docs: add new top level about page * fixup * fixup * fixup

dstansby mentioned this pull request Jan 2, 2025

Use doctests for guide #2623

Merged

Merge branch 'main' into docs/3.0-user-guide

a0cfc59

dstansby added 6 commits January 2, 2025 14:06

Add a doctest CI run

bf694c3

Remove rmtrees

8ecff3e

Delete data dir before doctests

80da829

Merge branch 'main' into docs/3.0-user-guide

3754466

Fix doctests

2dab20e

dstansby requested changes Jan 3, 2025

View reviewed changes

normanrz added 10 commits January 3, 2025 12:25

fix doctests for arrays.rst

5b5e757

fix doctests for consolidated_metadata.rst

c0ce409

fixes for doctest

2c57014

tests

9490001

debugging

5c41cfb

debugging

4e66548

debugging

74f06e8

debugging

01fa8a9

debugging

6142d6e

Merge branch 'main' into docs/3.0-user-guide

705d877

normanrz requested a review from dstansby January 3, 2025 14:15

dstansby approved these changes Jan 3, 2025

View reviewed changes

dstansby merged commit d6384f5 into main Jan 3, 2025
33 checks passed

dstansby deleted the docs/3.0-user-guide branch January 3, 2025 14:31

normanrz mentioned this pull request Jan 3, 2025

show pprint import #2632

Merged

jhamman mentioned this pull request Jan 4, 2025

DOC: zarr.tree() is deprecated #2550

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: split tutorial into multiple user guide sections #2589

docs: split tutorial into multiple user guide sections #2589

jhamman commented Dec 29, 2024 •

edited

Loading

dstansby left a comment

jhamman commented Jan 1, 2025

dstansby commented Jan 1, 2025 •

edited

Loading

jhamman commented Jan 1, 2025 •

edited

Loading

dstansby commented Jan 2, 2025

dstansby left a comment

jhamman commented Jan 2, 2025 •

edited

Loading

dstansby commented Jan 2, 2025

normanrz commented Jan 2, 2025 •

edited

Loading

d-v-b commented Jan 2, 2025

dcherian commented Jan 2, 2025

dstansby commented Jan 2, 2025 •

edited

Loading

d-v-b commented Jan 2, 2025

dstansby commented Jan 2, 2025 •

edited

Loading

jhamman commented Jan 2, 2025

dstansby commented Jan 2, 2025 •

edited

Loading

dstansby commented Jan 2, 2025

dstansby left a comment

normanrz commented Jan 3, 2025

dstansby left a comment

dstansby Jan 3, 2025

normanrz Jan 3, 2025

docs: split tutorial into multiple user guide sections #2589

docs: split tutorial into multiple user guide sections #2589

Conversation

jhamman commented Dec 29, 2024 • edited Loading

dstansby left a comment

Choose a reason for hiding this comment

jhamman commented Jan 1, 2025

dstansby commented Jan 1, 2025 • edited Loading

jhamman commented Jan 1, 2025 • edited Loading

dstansby commented Jan 2, 2025

dstansby left a comment

Choose a reason for hiding this comment

jhamman commented Jan 2, 2025 • edited Loading

dstansby commented Jan 2, 2025

normanrz commented Jan 2, 2025 • edited Loading

d-v-b commented Jan 2, 2025

dcherian commented Jan 2, 2025

dstansby commented Jan 2, 2025 • edited Loading

d-v-b commented Jan 2, 2025

dstansby commented Jan 2, 2025 • edited Loading

jhamman commented Jan 2, 2025

dstansby commented Jan 2, 2025 • edited Loading

dstansby commented Jan 2, 2025

dstansby left a comment

Choose a reason for hiding this comment

normanrz commented Jan 3, 2025

dstansby left a comment

Choose a reason for hiding this comment

dstansby Jan 3, 2025

Choose a reason for hiding this comment

normanrz Jan 3, 2025

Choose a reason for hiding this comment

jhamman commented Dec 29, 2024 •

edited

Loading

dstansby commented Jan 1, 2025 •

edited

Loading

jhamman commented Jan 1, 2025 •

edited

Loading

jhamman commented Jan 2, 2025 •

edited

Loading

normanrz commented Jan 2, 2025 •

edited

Loading

dstansby commented Jan 2, 2025 •

edited

Loading

dstansby commented Jan 2, 2025 •

edited

Loading

dstansby commented Jan 2, 2025 •

edited

Loading