Make fmt.version extensions backward compatible in ome-zarr-py dependents. - #491

cc @LucaMarconato @will-moore @joshmoore

I know there are other ways of doing this, but I think any other change would require a change in both ome-zarr-py and dependent libraries (though SpatialData could be the only one in this case effected)

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.02%. Comparing base (b49986e) to head (b5b85c5).
⚠️ Report is 28 commits behind head on master.

@@           Coverage Diff           @@
##           master     #491   +/-   ##
=======================================
  Coverage   87.02%   87.02%           
=======================================
  Files          13       13           
  Lines        1772     1772           
=======================================
  Hits         1542     1542           
  Misses        230      230           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Thanks for approving! Would there be any timeline for a release? Right now zarr v3 in SpatialData is blocked by this as we can't have backward compatibility without this.

Just to understand your usage a bit better... Your RasterFormatV02 is a subclass of FormatV04?

I'm wondering if doing if fmt.version.zarr_format == 2 might be a nicer way of checking for versions 0.1-0.4 in the future?

correct, so that would seemingly be an alternative right now, though I would not be able to predict if in the future we have a similar situation while the format is then still zarr_format == 3. Hopefully not, but not certain, so I think the current solution is better.

One possible issue in the future is that "0.10" startswith "0.1", although I expect we'll remove support for writing "0.1" by the time we get to "0.10".

As far as I can understand, following your PR link above, all your Format classes are based on CurrentFormat(), which is now based on zarr V3?

But in your RasterFormatV02 are you also nesting your metadata within an ome block?
It sounds like you don't want to unwrap the ome nesting as is happening here:

if not fmt.version.startswith(("0.1", "0.2", "0.3", "0.4")):
        attrs = attrs.get("ome", {})

So you have zarr.json and attributes: {} but this doesn't contain ome?

If so, then my suggestion of checking for fmt.zarr_format would certainly not work for you.

I'm just trying to work out the best way of not breaking your usage in future, but I don't know which of the checks was the one that broke your usage, or exactly why.
Would it be worth adding a test that checks the behaviour works for you?

Or maybe I'm over-thinking this and we can just go with the fixes as they are now.

Not all Format classes are based on current format. RasterFormatV02 is inheriting from RasterFormatV01 which is based on ome-zarr-py FormatV04. This does not contain the ome key in attrs. The main reason for supporting this raster format is for allowing users to read the data that they have in zarr v2 and convert it to zarr v3 by then writing it back to disk with the current format. All our current formats would be based on FormatV05, but can contain additional metadata besides ome.

TLDR We need this to allow our users to update their zarr v2 data to zarr v3. I don't foresee any problems once we have given the time for our users to update.

The main reason why it is broken now is that our RasterFormatV02 which was used with zarr v2 does not write metadata to ome. However, ome-zarr-py recognizes it as being of format ' 0.5' which assumes the ome key. This because our format version is 0.4-spatialdata-dev. This does not match either of the versions in the conditional.

Also, just for the record, thank you for thinking along:)

Hi there! I wonder if there's any progress on this? Or is there anything that I can help with?
This seems to be a blocker to enable zarr V3 support in spatialdata, which we'd like to try out ASAP.

I will close this PR, after talking yesterday together with @joshmoore , @jo-mueller and @LucaMarconato to discuss the potential implications and concerns of @sbesson, we have chosen for the option of overwriting the version metadata with the version as indicated by SpatialData. We know which Format is used with which element format so we can do the mapping on our side.

Prior to reading we then also check the metadata for the version and then request the corresponding ome-zarr version when calling ZarrLocation.

This leaves still the issue that there should be a way to deviate in case of ome development versions and that there should be a way to indicate something like:

1. which format is used for reading the data
2. if the version string is an exact match, we expect that the data gets validated by NGFFvalidator, if only partial match then not.

However, this is a RFC process.

Thanks @melonora for the summary. Reflecting a bit on the discussions I had with various OME members here, I believe what happened here was the outcome of a classical conflict between:

• the reference expectations for a library such as this one which mandates strict compliance with the specification especially on write
• the needs of the spatialdata library especially in terms of expanding the current specification

From OME's previous experience with formats, this type of tension is not specific to OME-Zarr by any means. In my opinion, the approach described in #491 (comment) i.e. use this library with a canonical OME-Zarr version and then let the SpatialData library override the metadata/binary data wherever applicable is a solid workaround which respects the requirements/needs of each library.

To the last point, I am very supportive of defining development OME-Zarr versions formally moving forward. The main concern was to effectively make these decisions implicitly by encoding them in a implementation. I think this topic deserves its own RFC. This might be very timely given the current discussions around the OME-Zarr 1.0 roadmap as I suspect another point to codift over the next few months will be the expectations in terms of compatibility between the different OME-Zarr versions.