Allow nested dictionaries in the Zarr backend (#3517)

[eddienko]

Closes #3517.

I have tried to touch the minimum code as possible while leaving the defaults as they are now. The code works now by redefining to value to save as a list containing the dictionary. However when writing to disk the the dictionary is saved correctly, without the list (I do not understand 100% how it works but it does so someone double check!).

$ cat test.zarr/.zattrs
{
    "foo": {
        "bar": {
            "val": 3
        }
    }
}

[dcherian]

Hmm.. This looks like we are inventing a convention to store nested dictionaries; is that right?

[eddienko]

I thought the same, but it is not the case. The nested dictionary is stored correctly in the .zattrs file, and by correctly I mean in the same way that using Zarr would store them. If we use Zarr directly to read the attributes I get a nested dictionary.

[rabernat]

I am slammed right now with a proposal and can't really spend time on this, so I'll tag @jakirkham and @alimanfoo who can provide a zarr perspective on what is going on here.

[jakirkham]

Yeah this probably works as these are just JSON files. That said, IDK that we are making any attempt to ensure this works. IOW I don't think this is tested or in the spec.

Additionally IDK that we do the same decoding on nested dictionaries as would be done on a flat dictionary. Meaning non-JSON values like datetime64/timedelta64 might not be handled correctly in this case.

Could be wrong about these things. Those are just my immediate thoughts.

[alimanfoo]

FWIW in the Zarr Python implementation I don't think we do any special encoding or decoding of attribute values. Whatever value is given then gets serialised using the built-in json.dumps. This means I believe that if someone provides a dict as an attribute value then that will get serialised as a JSON object, and get deserialised back to a dict, although this is not something we test for currently.

From the zarr v2 spec point of view I think anything goes in the .zattrs file, as long as .zattrs is a JSON object at the root.

Hth.

[dcherian]

Whatever value is given then gets serialised using the built-in json.dumps. This means I believe that if someone provides a dict as an attribute value then that will get serialised as a JSON object,

@eddienko I think this means we can avoid converting to a list?

[eddienko]

mmm, I am not sure I understand. Currently

ds = xr.Dataset()
ds.attrs['foo'] = {'bar': 1}
ds.to_zarr('foo.zarr', mode='w')

does not work. This is because dict is not allowed in the list of types that the attributes are validated against. My feeling is this should not produce an error.

This PR just addresses that so the code above works. The following works as well:

ds = xr.open_zarr('foo.zarr')
ds.attrs['foo']
{'bar': 1}

[jhamman]

Bring back this old issue since I think there is value in getting something like this in. My suggestion would be to write a zarr-specific _validate_attrs function and use that, rather than stubbing in a special case for zarr in the generic one. @eddienko , I know this has sat for a long time but would you like to try to finish this up?

[jhamman]

closing as stale. Folks should feel free to open a new PR to address the issue referenced above.