Specify NGFF 0.6-dev in output data #849
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #849 +/- ##
==========================================
+ Coverage 91.95% 91.96% +0.01%
==========================================
Files 47 47
Lines 7293 7304 +11
==========================================
+ Hits 6706 6717 +11
Misses 587 587
|
| pass | ||
| @property | ||
| def version(self) -> str: | ||
| return "0.6-dev" |
There was a problem hiding this comment.
cc @bogovicj in case defining a specific pre-release version (i.e. dev1 or beta1) would make detecting differences in these upcoming versions more reliable.
See also ongoing conversation around 0.5.2
There was a problem hiding this comment.
Actually, going back and looking at the diff output in the description, I'd suggest we not use 0.6.* unless this also implements the Zarr v3 work of 0.5.
There was a problem hiding this comment.
This is a good point. Any suggestion on the string to use? Maybe something more verbose like 0.4-dev-transformations?
There was a problem hiding this comment.
Or 0.4-dev-spatialdata .
There was a problem hiding this comment.
- 👍 for starting with
0.4- - 👍 for having "spatialdata" or similar in it
- you might think about whether or not you have a version field from SD itself
- I defer to you if you want "dev" in it or not (I don't think the validator, e.g., has special behavior for that. @will-moore?)
There was a problem hiding this comment.
Yes, I will adjust the coordinate transformations metadata together with @melonora in ome-zarr-models-py.
There was a problem hiding this comment.
If you agree, I'd go then with 0.4-dev-spatialdata and merge this PR.
There was a problem hiding this comment.
I've opened a PR at ome/ome-ngff-validator#45 with the validator deployed for testing at https://deploy-preview-45--ome-ngff-validator.netlify.app/
Does that help with what you need?
There was a problem hiding this comment.
Thanks Will, but as Josh said here:
Except this version explicitly does not validate, so perhaps it's enough that it's just not the known "0.4"?
Not passing the validation is expected because the coordinate transformation specification that we implemented was based on an earlier draft and our plan is to finalize it inome-zarr-models-py.
In this PR I made a patch to ome-zarr-py to be able to read the tensors even with partially incorrect metadata. We indeed handle the read/write of metadata in spatialdata, and use ome-zarr-py to read the tensors.
What would be needed here is to agree on an appropriate version number, such as 0.4-dev-spatialdata, that doesn't imply that validation is passed.
There was a problem hiding this comment.
Please in this comment you can see the full context of why we want to fix the version string now and we defer passing the validation later: #805 (comment).
|
As I understand it from #805, the eventual goals are:
The main issues are:
Re: your patch for Is this what you're asking for agreement on? I don't see a problem with that cc @joshmoore ? |
|
Correct @will-moore, thanks for the clear summary.
I prefer to direct my efforts in implementing the NGFF RFC-5 in @joshmoore wdyt? |
This PR monkeypatches
ome_zarr.formatto make it possible to use the read/write APIs ofome-zarr-py, while clarifying which OME-Zarr data version we are building on top.Precisely, we implement the NGFF transformations specification, which, as of today, it is not merged to NGFF main yet. The timeline is to have the specification included in NGFF v0.6, hence from this PR we will write
0.6-devinstead of0.4(which was kept for compatibility withome-zarr-py).All the tests pass, the code for reproducing the documentation and paper notebooks and the data conversions from
spatialdata-sandboxare currently running.Practically, the PR will not change APIs or usage of the function of the
spatialdatapackage and will only modify, on-disk, the version string for images and labels, as shown here (in this example I used Seqfish v2 data).CC @joshmoore