feat(omezarr): store a displacement field as an NGFF RFC-5 vector field - #67
Merged
Conversation
The OME-Zarr backend could only store images, so a registration DVF went out as an anonymous 3-channel volume: nothing on disk said the channels were a displacement, and a reader indexing the component axis like any other got a third of the field back -- and a plausible-looking registration with it. Dataset.write now accepts a DisplacementFieldTransform on this backend, as the H5 and SimpleITK ones already accept a Transform, and types the component axis "displacement" (NGFF RFC-5). read_transform recognises such a store and rebuilds the transform: the counterpart of _decode_transform for the one kind whose parameters ARE the field, and which therefore cannot travel as a parameter vector without losing the geometry that gives it meaning. The NGFF version is not a parameter. RFC-5 axis types exist only from 0.6, so it follows from the request instead of being an invariant for callers to keep in sync. The type is set on the coordinate systems rather than on the NgffImage: to_multiscales derives the axes itself and hardcodes type="channel" for a "c" dim, so tagging the image is a dead assignment that writes an untyped store and raises nothing. Their absence doubles as the capability check, hence ngff-zarr >= 0.38.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds RFC-5 OME-Zarr support for SimpleITK displacement-field transforms, including metadata detection, image-backed serialization, reconstruction, dependency constraints, and unit coverage for round trips and invalid inputs. ChangesOME-Zarr displacement field support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Dataset
participant OmeZarrFile
participant write_ome_zarr
participant SimpleITK
Dataset->>OmeZarrFile: write DisplacementFieldTransform
OmeZarrFile->>write_ome_zarr: displacement image and RFC-5 mode
write_ome_zarr->>OmeZarrFile: RFC-5 OME-Zarr store
Dataset->>OmeZarrFile: read tagged displacement field
OmeZarrFile->>SimpleITK: vector displacement image
SimpleITK->>Dataset: restored DisplacementFieldTransform
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_omezarr_displacement_field.py (1)
42-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover direction round-tripping.
The fixture leaves Direction at identity and the round-trip test never asserts it, so a lost or transposed direction matrix would pass despite violating the geometry contract.
Proposed test update
SPACING = (1.5, 1.5, 2.0) ORIGIN = (7.0, -3.0, 10.0) +DIRECTION = (0.0, -1.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0) def _displacement_field_transform() -> "sitk.DisplacementFieldTransform": ... field.SetSpacing(SPACING) field.SetOrigin(ORIGIN) + field.SetDirection(DIRECTION) return sitk.DisplacementFieldTransform(sitk.Cast(field, sitk.sitkVectorFloat64)) ... assert after.GetOrigin() == pytest.approx(ORIGIN) + assert after.GetDirection() == pytest.approx(DIRECTION)As per coding guidelines, preserve geometry using
Origin,Spacing, andDirection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_omezarr_displacement_field.py` around lines 42 - 80, Update _displacement_field_transform to assign a non-identity, anisotropic Direction matrix, then extend test_transform_round_trips_through_the_store to assert the restored displacement field’s Direction matches the original using an approximate comparison alongside Origin and Spacing.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@konfai/utils/dataset.py`:
- Around line 314-326: Redesign displacement-field serialization and loading
around lazy, patch-based or streaming access rather than full-volume
materialization. Update displacement_field_to_data and the displacement-field
branches of read_transform and related write/read paths to avoid image_to_data
and read_data for complete volumes, using can_stream_patch, read_data_slice, or
a lazy transform representation while preserving geometry and transform
behavior.
---
Nitpick comments:
In `@tests/unit/test_omezarr_displacement_field.py`:
- Around line 42-80: Update _displacement_field_transform to assign a
non-identity, anisotropic Direction matrix, then extend
test_transform_round_trips_through_the_store to assert the restored displacement
field’s Direction matches the original using an approximate comparison alongside
Origin and Spacing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9592718-6b8a-4c57-a16c-76ca217a27df
📒 Files selected for processing (4)
konfai/utils/dataset.pykonfai/utils/ome_zarr.pypyproject.tomltests/unit/test_omezarr_displacement_field.py
RFC-5 displacement fields are an NGFF >= 0.5 store, i.e. zarr v3, which needs Python >= 3.11. The capability check typed the component axis off coordinateSystems -- an ngff-zarr signal, not a zarr one -- so on zarr 2.x (Python 3.10) it let the write through to an opaque ValueError from ngff-zarr. This was the whole 3.10 CI failure. - write_ome_zarr now raises a clear DatasetManagerError when zarr v3 is unavailable, via a _zarr_v3_available() capability helper - the RFC-5 displacement tests skip where zarr v3 is unavailable (Python 3.10), the feature genuinely not existing there - the round-trip test uses a non-identity Direction and asserts it survives
vboussot
force-pushed
the
feat/omezarr-rfc5-displacement
branch
from
July 28, 2026 15:10
92571f0 to
8d75f72
Compare
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
1 participant
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.
Why
The OME-Zarr backend could only store images. A registration DVF therefore went out as an anonymous 3-channel volume: nothing on disk said the channels were a displacement, nor which way it pointed.
The failure that invites is silent rather than loud — index the component axis like any other and you get a third of the field back, and a plausible-looking registration with it. Downstream code had to be told out of band what the store held, so in practice DVFs kept travelling as
.mha.What
Displacement fields are stored as NGFF RFC-5 vector fields: the component axis is typed
displacement, so the store is self-describing.This follows the shape the other backends already use rather than adding a mechanism beside them —
Dataset.writeaccepts aTransform, dispatch is byisinstance, and read/write are symmetric:sitk.WriteTransform(...)→.itk.txt_decode_transform+ parameter vectorDisplacementFieldTransformfrom the fieldread_transformgains the displacement case: the counterpart of_decode_transformfor the one transform kind whose parameters are the field, and which therefore cannot travel as a parameter vector without losing the geometry that gives it meaning.Nothing to declare: the type of the data decides, and the store — not the producer — is what is trusted on read.
Two traps worth recording
The axis type goes on the coordinate systems, not on the
NgffImage.to_multiscalesderives the axes itself and hardcodestype="channel"for acdim, so tagging the image is a dead assignment on a non-frozen dataclass: the store comes out an ordinary 3-channel image and nothing raises. Verified end-to-end before settling on the coordinate-system path.The NGFF version is not a parameter. RFC-5 axis types exist only from 0.6, so a caller passing both could only ever pass them consistently — an invariant worth removing rather than documenting.
0.4stays the default for every other write. The absence ofcoordinateSystemsdoubles as the capability check, which is whyngff-zarrmoves to>= 0.38.Tests
tests/unit/test_omezarr_displacement_field.py— the axis type reaches disk, the transform round-trips with its geometry, a plain 3-channel image is not mistaken for a field, a missing store answers rather than raises, and a parametric transform is rejected loudly.The round-trip is asserted on behaviour, not only on the array: a field read back transposed, or with one component selected, still has a plausible shape — only applying it to a point catches that.
ruff,ruff format,mypy,banditclean; existing dataset / imaging / streaming suites pass.Summary by CodeRabbit
displacement_fieldoption for the OME-Zarr writer to emit displacement-field representations.ngff-zarrminimum version requirements for relevant optional dependency groups.