Conversation
When ome_version is 0.6, restructure the multiscales metadata per RFC-5:
emit a named `coordinateSystems` array (the "intrinsic" physical system)
in place of the top-level `axes` key, and give each dataset scale
transform explicit `input` (the array coordinate system, i.e. the dataset
path) and `output` ("intrinsic"). 0.5 output is unchanged.
Pure emission change gated on ome_version; no new public API. Also adds
the RFC-5 design doc under docs/design/.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RFC-5 coordinate transformations reference their input and output coordinate
systems as objects -- {"path": ...} for an array and {"name": ...} for a
named system -- not as bare strings.
MSVC resolves `json = { <json prvalue> }` to copy-assignment, so the datasets
key became the single dataset object rather than an array containing it, and
the next level's push_back threw type_error.308. Clang and GCC pick the
initializer-list constructor and produce the array, so this only broke the
MSVC build -- and PRs stacked on a non-main base get no CI at all, so nothing
caught it.
Use nlohmann::json::array() explicitly, which is unambiguous on both.
Member
|
I worry about the approach on this one. Ideally, we wouldn't need to explicitly model every ngff feature. For example, if the caller can just hand us a json string that we put in the right place (like an "omero" key), then we don't need to model hardly anything here. That way we don't need to change the code as much as the standard evolves. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second slice of the OME-NGFF expansion: emit RFC-5 coordinate systems and
transformations when the stream selects OME-Zarr 0.6.
Stacked on #240 — that PR adds the
ome_versionselector this one gates on.What changes
MultiscaleArray::make_multiscales_metadata_()restructures the existingper-level
scaletransform into the RFC-5 shape whenome_version == 0_6:coordinateSystemsentry named"intrinsic", built from the visible axesscaletransforms that name theirinput({"path": "<level>"})and
output({"name": "intrinsic"})axesin 0.6, whichcoordinateSystemssupersedes0.5 output is unchanged. No new user-facing API — richer transforms
(translation/affine/rotation/sequence, named systems) need one and come later.
Design notes and the resolved open questions are in
docs/design/rfc5-coordinate-transformations.md.Notes
RFC-5 is still under review, so 0.6 stays opt-in and is pinned to 0.6.dev3.
Testing
tests/integration/stream-omero-and-ome-version.cppasserts the 0.6 shape andkeeps the 0.5 assertions as a regression guard
python/tests/test_stream.py::test_ome_version_selector