Add mode="r+" for to_zarr and use consolidated writes/reads by default

[shoyer]

mode="r+" only allows for modifying pre-existing array values in a
Zarr store. This makes it a safer default mode when doing a limited
region write. It also offers a nice performance bonus when using
consolidated metadata, because the store to modify can be opened in
"consolidated" mode -- rather than painfully slow non-consolidated
mode.

This PR includes several related changes to to_zarr():

1. It adds support for the new mode="r+".
2. consolidated=True in to_zarr() now means "open in consolidated
   mode" if using using mode="r+", instead of "write in consolidated
   mode" (which would not make sense for r+).
3. It allows setting consolidated=True when using region, mostly
   for the sake of fast store opening with r+.
4. Validation in to_zarr() has been reorganized to always use the
   existing Zarr group, rather than re-opening zar stores from
   scratch, which could require additional network requests.
5. Incidentally, I've renamed the ZarrStore.ds attribute to
   ZarrStore.zarr_group, which is a much more descriptive name.

These changes gave me a ~5x boost in write performance in a large
parallel job making use of to_zarr with region.

• Tests added
• Passes pre-commit run --all-files
• User visible changes (including notable bug fixes) are documented in whats-new.rst

[max-sixty]

🙌

[rabernat]

Question: does this mode still require eager loading of dimension coordinates?

[shoyer]

Nope, not anymore! We decide the existing variables (to figure out/match their encoding) but that's done in a lazy way. We never put them in an xarray.Dataset so there is no eager loading of dimension coordinates.

…

On Tue, May 4, 2021 at 7:07 AM Ryan Abernathey ***@***.***> wrote: Question: does this mode still require eager loading of dimension coordinates? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5252 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJJFVVIPHXHCNFR2GJXJKDTL75R5ANCNFSM44ALL2AQ> .

[shoyer]

I pushed a commit making consolidated metadata the default (see #5251). This was easier to do building on top of my existing refactor here (but conceivably could be separated).

[shoyer]

Thanks @joshmoore for the copy-editing!

[pep8speaks]

Hello @shoyer! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-06-17 16:06:29 UTC

[shoyer]

@rabernat gentle ping for review here :)

[shoyer]

@nbren12 @spencerkclark are either of you up for a review here?

[dcherian]

I vote to merge since it's so useful and @shoyer has probably found all the bugs already :)

[shoyer]

I vote to merge since it's so useful and @shoyer has probably found all the bugs already :)

Hah :). We've been using this branch at Google for a while now and it seems to be working pretty well.

[dcherian]

it seems to be working pretty well.

best kind of code review!

Thanks @shoyer

[github-actions]

Unit Test Results

0 files  ±0  0 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 158bc52. ± Comparison against base commit 158bc52.

[rabernat]

Really sorry I didn't get around to review. My excuse is that I moved back to NYC last week and fell behind on everything. Thanks for moving it forward. 💪

[nbren12]

I'm sorry too! I don't have any good excuse though...

[shoyer]

Well hey, it's still not too late to catch any issues before they turn up in a released version of Xarray :)

…

On Mon, Jun 21, 2021 at 11:56 PM Noah D. Brenowitz ***@***.***> wrote: I'm sorry too! I don't have any good excuse though... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5252 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJJFVXBEQCIEIUDKLD6NBLTUAJXLANCNFSM44ALL2AQ> .