Add shards to valid_encodings to enable sharded Zarr writing

[jacobbieker]

Adds shards to the list of valid_encodings in the zarr backend, so that sharded Zarr V3s can be written.

• Closes Unable to write to sharded Zarr V3 because shards not in valid_encodings #9947

[welcome]

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

[jhamman]

Thanks @jacobbieker for opening a PR. Do you have time to add a roundtrip test for this? This test would be a good test to emulate:

xarray/xarray/tests/test_backends.py

Lines 2486 to 2494 in 6bea715

	def test_chunk_encoding(self) -> None:
	# These datasets have no dask chunks. All chunking specified in
	# encoding
	data = create_test_data()
	chunks = (5, 5)
	data["var2"].encoding.update({"chunks": chunks})
	with self.roundtrip(data) as actual:
	assert chunks == actual["var2"].encoding["chunks"]

[jacobbieker]

Thanks @jacobbieker for opening a PR. Do you have time to add a roundtrip test for this? This test would be a good test to emulate:

xarray/xarray/tests/test_backends.py

Lines 2486 to 2494 in 6bea715

	def test_chunk_encoding(self) -> None:
	# These datasets have no dask chunks. All chunking specified in
	# encoding
	data = create_test_data()
	chunks = (5, 5)
	data["var2"].encoding.update({"chunks": chunks})
	with self.roundtrip(data) as actual:
	assert chunks == actual["var2"].encoding["chunks"]

Yep! I think I've added one that works for that, and updated the Zarr V3 loading to include the shards. It seems to pass the tests locally so far at least.

Edit: Ah it seems to cause some issues with threads on 3.12 potentially?

[jhamman]

I seem to remember fixing this in the Dask test suite by manually cleaning up thread resources. This may help: https://github.com/dask/dask/blob/e04734b4d8959ba259801f2e2a490cb4ee8d891f/dask/tests/test_distributed.py#L338-L358

[kmuehlbauer]

@jhamman The dask distributed zarr integration test is also failing on main. Would be great to have a fix applied.

[kmuehlbauer]

I seem to remember fixing this in the Dask test suite by manually cleaning up thread resources. This may help: https://github.com/dask/dask/blob/e04734b4d8959ba259801f2e2a490cb4ee8d891f/dask/tests/test_distributed.py#L338-L358

See #9967

[jacobbieker]

I seem to remember fixing this in the Dask test suite by manually cleaning up thread resources. This may help: https://github.com/dask/dask/blob/e04734b4d8959ba259801f2e2a490cb4ee8d891f/dask/tests/test_distributed.py#L338-L358

See #9967

@jhamman yeah, this change in #9967 fixes the tests and the failing ones pass.

[jhamman]

Thanks for this @jacobbieker !

[jacobbieker]

Thanks for this @jacobbieker !

No problem! Excited to use this a lot! I am not able to merge this PR, is it possible you can? Its saying I'm not authorized.

[jhamman]

We'll let @dcherian do a final review and/or merge.

[dcherian]

Looks great. Thanks @jacobbieker, welcome to Xarray!

[welcome]

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again!

[jhamman]

Thanks @jacobbieker for the contribution here!