Fix structured arrays that contain objects #806

[abergou]

• Ensures that the fill value of structured arrays that contain objects
  is encoded using object_codec.

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[joshmoore]

Hi @abergou. Thanks for the PR! I've kicked off the github actions.

It looks like this may be a more complete version of #702. Do you mind taking a look and seeing what you think?

[abergou]

Sure! Thanks! will take a look tomorrow morning hopefully that's quick enough.

[joshmoore]

hopefully that's quick enough.

Definitely! Cheers.

[abergou]

Thanks @joshmoore! Indeed this pull request is a more complete version of #702 . The latter fixes part of the problem, but the fill_value still needs to be encoded by the object_codec to fix the issue for structured arrays.

[abergou]

The last patch fixes up some failing unit tests; the remaining unit test failures are not due to this patch. They also appear against the master branch.

[joshmoore]

Failures were due to fsspec/s3fs#513 ; ready for retesting with #812 merged.

[codecov]

Codecov Report

Merging #813 (0088fa4) into master (e095fe5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #813   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          31       31           
  Lines       10609    10680   +71     
=======================================
+ Hits        10603    10674   +71     
  Misses          6        6           

Impacted Files	Coverage Δ
zarr/codecs.py	100.00% <100.00%> (ø)
zarr/meta.py	100.00% <100.00%> (ø)
zarr/storage.py	100.00% <100.00%> (ø)
zarr/tests/test_core.py	100.00% <100.00%> (ø)
zarr/tests/test_meta.py	100.00% <100.00%> (ø)
zarr/util.py	100.00% <100.00%> (ø)

[abergou]

@joshmoore there is a linting error that is due to this pr; it's purely cosmetic though (i.e. it's an error condition that will not show up unless encode_fill_value/decode_fill_value are called directly), but I'll quickly get together a fix.

[joshmoore]

Pushed an attempted codecov fix. Feel free to adapt.

[abergou]

Thanks @joshmoore your changes look good.

I'm confused about the linting reported by the test suite above: zarr.codecs.Pickle is definitely there so it appears to be a false positive? Aside from this issue I think this is ready to merge.

[joshmoore]

Pushed a suggested fix. (Really need to turn on pre-commit for this repo.)

[joshmoore]

Green! Is there anyone familiar with object codec'ing that would like to take a look at this?

[joshmoore]

I've resurrected the test from #702, @abergou, as one extra check.

Note: it does not pass with fsspec 2021.07.0.

[abergou]

All green! Thanks @joshmoore!

Do you know when this might be merged?

[joshmoore]

Bumped the release notes to point to the (imminent!) 2.9.4. Any last thoughts from the community?

[joshmoore]

Didn't receive any objections over the weekend. Releasing.