Discussion: Change default codec for hdmf_zarr_utils.ZarrReference

[ehennestad]

From documentation:

By default, ZarrIO uses the numcodecs.pickles.Pickle codec to encode object references defined as py:class:~hdmf_zarr.utils.ZarrReference dicts in datasets. Users may set the codec used to encode objects in Zarr datasets via the object_codec_class parameter of the init() constructor of ZarrIO. E.g., we could use ZarrIO( ... , object_codec_class=numcodecs.JSON) to serialize objects using JSON.

I am working on adding read support for Zarr files in MatNWB. It seems to me that using the numcodecs.pickles.Pickle codec for storing hdmf_zarr.utils.ZarrReference objects adds extra hurdles for reading zarr files with other languages than Python.

I wonder if it would be better to use numcodecs.JSON by default, and maybe even remove the option to set a codec via parameter?

[bendichter]

@rly would be interested in your thoughts on this. Is it a priority to ensure these files are readable by other languages?

[oruebel]

I wonder if it would be better to use numcodecs.JSON by default, and maybe even remove the option to set a codec via parameter?

At least for NWB, I think choosing one codec would be useful. It's been a while since I've hacked on the codec part of hdmf-zarr but as far as I recall we had used the pickle codecmainly because it is the most broad and reliable in terms of its ability to serialize Python objects. The JSON codec had issues with serializing certain data types and using pickle was the simplest solution at the time. I don't recall which specific data types where causing issues, but I think structured arrays was one of them. However, a lot of things have changed since then so its worth re-evaluating that choice.

As a first step, I think it would be useful to understand whether the JSON codec supports serializing all the types we need. Changing the codec is a fairly minimal code change so I created a draft PR to test this here #286 so we can see which tests will fail when using JSON as the codec.

It looks like there are a few errors that happen in the JSON codec:

• E ValueError: setting an array element with a sequence. The requested array would exceed the maximum number of dimension of 1.
• E TypeError: Object of type void is not JSON serializable

These errors happen several times, but at first glance, I think these are likely repeats of the same issue, because they are triggered in the base test class, so the assert is run several times under different conditions.

I am not sure right now how hard it would be to fix those errors and make JSON work as the default codec. We would need to dig a bit deeper to see what actually triggers those errors.

maybe even remove the option to set a codec via parameter?

I think for ZarrIO I would probably leave the object_codec_cls option in place and remove it only from NWBZarrIO (i.e., enforcing the codec only for NWB).