Breaking change to Kerchunk references spec output #679

The Kerchunk references spec has a weird property, and we might want to write out kerchunk references from VirtualiZarr differently to avoid it.

The Kerchunk v0 spec describes how keys and values (for example zarr (v2) keys) are to be laid out within a dict. The Kerchunk v1 spec wraps this up into a larger spec with another layer of dict. The problem is in the v0 part.

The Kerchunk v0 spec allows for two different ways to format any dict values that themselves have multiple key-value pairs:

• as more dict objects, so the whole thing is truly JSON-serializable, like this:

  {
    ".zgroup": {"zarr_format": 2},
    ".zattrs": {"Conventions": "UGRID-0.9.0\n"},
    "x/.zattrs": {"_ARRAY_DIMENSIONS": ["node"]},
    "x/.zarray": {"chunks": [9228245], "compressor": null, "dtype": "<f8"},
    "x/0": ["s3://bucket/path/file.nc", 294094376, 73825960]
  }
  

• that same information with the first layer of values serialized to strings, like this:

  {
    ".zgroup": "{\n    \"zarr_format\": 2\n}",
    ".zattrs": "{\n    \"Conventions\": \"UGRID-0.9.0\n\"}",
    "x/.zattrs": "{\n    \"_ARRAY_DIMENSIONS\": [\n        \"node\"\n ...",
    "x/.zarray": "{\n    \"chunks\": [\n        9228245\n    ],\n    \"compressor\": null,\n    \"dtype\": \"<f8\",\n  ...",
   "x/0": ["s3://bucket/path/file.nc", 294094376, 73825960]
  }
  

Currently VirtualiZarr writes out the latter form, and some of our tests expect that. I think we (I) arbitrarily chose this form, not even fully understanding the Kerchunk spec at the time.

Now though, I can see that these is a problem with this partially-string-serialized form: the order of the keys in the string-serialized parts matter.

As @d-v-b points out in zarr-developers/zarr-python#3254, order is not supposed to matter in JSON. But two programs aiming to produce this spec format by first creating intermediate JSON objects and serializing them (i.e. treating them as true JSON) aren't guaranteed to end up with exactly compatible serialized string results, because the order of the keys in the string parts can end up different. This has now happened in VirtualiZarr (part of #677) due to upstream changes in Zarr's metadata classes, which reasonably assume order doesn't matter (see zarr-developers/zarr-python#3254).

This means the change in zarr-python, which by rights shouldn't matter, has technically caused a breaking change in the format that VirtualiZarr's .to_kerchunk writes out, which is only breaking because of an ambiguity in the Kerchunk references spec.

We have four choices I can see:

1. Try to get zarr-python to preserve order of metadata keys (I don't think we should do that because JSON doesn't require that).
2. Keep writing the same way in VirtualiZarr, adjust our tests slightly, and hence ignore the fact our output has changed. This may be okay - if everyone else using these references (i.e. likely just fsspec/kerchunk) is first deserializing them to dicts piecemeal, then the problem washes out.
3. Manually alter the order of keys within the serialized string part before returning. This would be a weird hacky thing to do but would avoid altering the output and avoid changing tests. It would effectively be treating key order as an important part of the Kerchunk v0 spec to be respected.
4. Change the format that VirtualiZarr's .to_kerchunk writes out to use the true JSON version of the Kerchunk v0 spec (i.e. fully-nested dicts). This is the "correct" solution from a data model perspective, but is also breaking, and I don't know if changing it will surface any other bugs.

I'm inclined to do (3) for now and keep this issue open in case we need to do (4) later (or in case kerchunk/fsspec needs to make any changes to handle the ambiguity).

cc @martindurant in case you have thoughts