Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

omero metadata structure disappeared from version 0.5 (accidentally?) #286

Closed
dstansby opened this issue Jan 8, 2025 · 6 comments
Closed

Comments

@dstansby
Copy link
Contributor

dstansby commented Jan 8, 2025

We noticed over in ome-zarr-models-py (ome-zarr-models/ome-zarr-models-py#91) that this block of text from verison 0.4 of the text:

The "omero" metadata is optional, but if present it MUST contain the field "channels", which is an array of dictionaries describing the channels of the image.
Each dictionary in "channels" MUST contain the field "color", which is a string of 6 hexadecimal digits specifying the color of the channel in RGB format.
Each dictionary in "channels" MUST contain the field "window", which is a dictionary describing the windowing of the channel.
The field "window" MUST contain the fields "min" and "max", which are the minimum and maximum values of the window, respectively.
It MUST also contain the fields "start" and "end", which are the start and end values of the window, respectively.

Has gone in verison 0.5. @normanrz and @joshmoore I think have both said this was an accident, so I'm opening this issue to track potentially putting it back in the next version of the spec.

From an implementors point of view, 🙏 I would like to strongly advocate that verison 0.5 isn't edited to put this back in. Now 0.5 is released it should be final, and any changes should happen in a new versioned release, especially additions to the specificaiton which fixing this issue would be. This way everyone agrees on what version 0.5 of the spec is, without it silently changing.

@normanrz
Copy link
Contributor

normanrz commented Jan 8, 2025

Thanks for noticing this issue.

I would strongly advocate for fixing the 0.5 release. This was clearly an unintended editorial accident. Fixing that doesn't seem like a big deal. In fact, not fixing it would create undue burden for implementors, who (correctly) didn't remove support for the omero key in 0.5.

@d-v-b
Copy link
Contributor

d-v-b commented Jan 8, 2025

if the spec is going to change after it is released, then minor versioning of the document should probably be introduced to make the history of changes explicit.

@d-v-b
Copy link
Contributor

d-v-b commented Jan 8, 2025

also, this seems like the perfect time to ask why we keep the separate versions of the spec as separate documents in the same repo, instead of using git to track versions and keeping old versions of the spec in the commit history. I think a linear history of changes would have prevented this issue entirely.

@dstansby
Copy link
Contributor Author

dstansby commented Jan 8, 2025

not fixing it would create undue burden for implementors, who (correctly) didn't remove support for the omero key in 0.5.

This isn't true - the text isn't in the released spec, so it would be incorrect for implementors to include it in their support for version 0.5. The whole point of having a specification is it is a common and fixed document that everyone implements from, and if the specification says (or doesn't say) something then that's the specificaiton, regardless of intent.

I would strongly advocate for fixing the 0.5 release.

👍, but 0.5 is released, so any changes should be made in a new released version. A release of the specification should not be changed at all, otherwise there's not a single text everyone can agree is verison 0.5. The new verison could be 0.5.1, as long as there's a clear changelog explaining what's changed.

@dstansby
Copy link
Contributor Author

dstansby commented Jan 8, 2025

This isn't true - the text isn't in the released spec, so it would be incorrect for implementors to include it in their support for version 0.5.

Unless, of course, we should be taking the JSONschema as the spec, and not the text - but my interpretation is that the text is the ground truth, and the JSONSchema an example implementation of the specification text.

joshmoore added a commit to joshmoore/ngff that referenced this issue Jan 10, 2025
@joshmoore
Copy link
Member

Going back through the history:

  • PRs that updated 0.4 and latest together: #218 #176 #112 #130 #126 #106 #97
  • PRs that have a partner on latest: #120 (#122), #119 (#118), #98 (#110)
  • PRs that only updated 0.4 and so were lost: #191

I've opened #287 to restore this as 0.5.1. This follows the strategy from 0.4.1 where a significant change was made to the spec that did not update the on-disk format but instead captured the requirements of one of the transitional blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants