fix(py): normalize generated axes to the spec order (t, c, z, y, x) - #623
Conversation
|
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 (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThe change normalizes supported image dimensions to OME-Zarr’s ChangesCanonical Axis Order
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant InputImage
participant to_multiscales
participant canonical_axis_order
participant DaskArray
InputImage->>to_multiscales: provide image and chunk mapping
to_multiscales->>DaskArray: convert image data
to_multiscales->>canonical_axis_order: normalize dimensions
canonical_axis_order->>DaskArray: apply lazy transpose
to_multiscales->>to_multiscales: re-key chunks in normalized order
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
78ef31e to
dbc3a39
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 53 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@py/test/test_issue_436.py`:
- Around line 146-153: Update the assertions in test_issue_436 around scale0 and
arr to validate the complete expected (c, y, x) shape, including the leading RGB
channel length from base, while preserving the existing y and x dimension
checks.
🪄 Autofix
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: 741562cf-c791-434b-8306-23667a354cf3
📒 Files selected for processing (9)
py/ngff_zarr/methods/_support.pypy/ngff_zarr/tiff_to_ngff_image.pypy/ngff_zarr/to_multiscales.pypy/test/test_bin_shrink_map_blocks_fast_path.pypy/test/test_canonical_axis_order.pypy/test/test_issue_436.pypy/test/test_large_image_chunking.pypy/test/test_tiff_pyramid_rgb.pypy/test/test_to_ngff_zarr_itkwasm.py
OME-Zarr orders axes by type: time, then channel, then space. ngff-zarr itself produced channel-last stores -- the TIFF S (sample) axis, ITK component images and the 4-D/5-D default dims inference all yield `(..., c)`, and the downsampling methods preserve the input order. `to_multiscales` and the pyramidal-TIFF reuse path now normalize the axis order with a lazy transpose before generating metadata, so every written store orders time, then channel, then space. In-memory `NgffImage` layouts are unrestricted as before; dims outside the canonical set pass through untouched, leaving RFC-3 axis models alone. Behavior change: converted RGB/vector data is now written `(c, z, y, x)` instead of `(z, y, x, c)`. Reading existing channel-last stores is unchanged.
84d8bdb to
0bbbd5b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
… order Merging main brought fideus-labs#623, which normalizes generated axes to the spec order (t, c, z, y, x). The brain_two_components DASK_IMAGE_GAUSSIAN baseline in the v0.21.0 testing-data archive was generated before that change, with the component axis last, so the key sets no longer match. Regenerate that baseline against the merged code and pin the updated archive.
… order Merging main brought #623, which normalizes generated axes to the spec order (t, c, z, y, x). The brain_two_components DASK_IMAGE_GAUSSIAN baseline in the v0.21.0 testing-data archive was generated before that change, with the component axis last, so the key sets no longer match. Regenerate that baseline against the merged code and pin the updated archive.
…r MUST Axis order is a spec MUST and validate_structural rejects it, so the writer rejects it too. The gate refuses exactly what the validator refuses. fideus-labs#623 normalizes generated axes to (t, c, z, y, x), so the Python pipeline produces a canonical order and a model that reaches the gate out of order comes from the caller. That normalization is Python only. Two TypeScript downsampling tests feed `(z, y, c, x)` and `(t, z, y, c, x)` to check that the channel axis survives in place, and write at 0.9.dev1, whose axis model allows the order they assert.
Addresses the channel-last axis order flagged in #611.
OME-Zarr (0.4–0.6) orders axes by type: time, then channel, then space.
ngff-zarr itself produced channel-last stores — the TIFF
S(sample) axis,ITK component images, and the 4-D/5-D default dims inference all yield
(..., c), and the downsampling methods preserve their input order. Theresult: stores that
validate_structuralitself flags withaxis-order,hence the write-gate carve-out in #611 that warns instead of raising.
What changes
to_multiscales()normalizes the axis order to(t, c, z, y, x)with alazy transpose before generating metadata; the pyramidal-TIFF reuse path
(which bypasses
to_multiscales) does the same per level.models (fix(py,ts): support OME-Zarr RFC-3 (arbitrary axis dimensions, names, types, ordering) #611) are unaffected.
NgffImagelayouts remain unrestricted; reading existingchannel-last stores is unchanged.
Behavior change
Converted RGB/vector data is written
(c, z, y, x)instead of(z, y, x, c)— the spec's MUST, and what bioformats2raw writes for RGB.Chunking keeps
cwhole, so each output chunk is a locally transposedinput tile; the transpose stays lazy.
Tests
test_canonical_axis_order.py: channel-last input, 4-D default dims,canonical passthrough, dim-keyed metadata integrity, unknown-dims
inertness.
orders, bin-shrink fast path, pyramidal RGB TIFF, chunk preservation,
issue-436 store shapes).
backend produces channel-last there).
Once this and #611 land, the axis-order write gate's warning only concerns
hand-built metadata.
Summary by CodeRabbit