Zstd: Don't persist the checksum param if false

[normanrz]

Changes the zstd codec to not persist the checksum param, if it is set to False (the default). This is aimed at improving backwards compatibility.

See zarr-developers/zarr-python#2647 (comment)

• Unit tests and/or doctests in docstrings
• Tests pass locally
• Changes documented in docs/release.rst
• Docs build locally
• GitHub Actions CI passes
• Test coverage to 100% (Codecov passes)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.92%. Comparing base (6c0ea0f) to head (93584af).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #681   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          62       62           
  Lines        2752     2756    +4     
=======================================
+ Hits         2750     2754    +4     
  Misses          2        2           

Files with missing lines	Coverage Δ
numcodecs/tests/test_zstd.py	100.00% <100.00%> (ø)

[normanrz]

@dstansby Do you know why the macos tests are failing? I don't have that issue on my local mac.

[dstansby]

It's the same failure at #679, so not related to either PR as far as I can see.

[dstansby]

re. this PR, I'm 👎 to changing the way this parameter is serialised, because it makes the serialisation logic more complex and potentially unituitive.

I think the right fix to zarr-developers/zarr-python#2647 is special casing how zarr chooses to seralise the zstd configuration when reading/writing zarr v2 data.

[normanrz]

I don't understand how special-casing in the client library (zarr-python) is any less complex than this small fix?

I don't know what numcodecs' policy for backwards compatibility is, but conditionally adding parameters seems like a good way of dealing with that.

[dstansby]

Yes, complexity of the fix would be the same, but numcodecs is used by a much wider range of packages than just zarr-python. Because this change introduces non-intuitive behaviour (only serialising a compression parameter if it has a particular value) that is not done for any other codecs in numcodecs, it should be down to zarr-python to choose a different way of serialising if it wants to.

[normanrz]

but numcodecs is used by a much wider range of packages than just zarr-python.

Is it really?

[dstansby]

2900 repostories and 136 packages according to GitHub: https://github.com/zarr-developers/numcodecs/network/dependents

[normanrz]

I suppose most of them have a transitive dependency through zarr.

Anyways. I don't care too strongly about this issue.

[dstansby]

In that case shall we close this, and I can add a note to zarr-developers/zarr-python#2647 that a fix/change to the codec serialisation should go in zarr-python?

[normanrz]

Closing in favor of zarr-developers/zarr-python#2655