feat(xarray): improve canonical fUSI DataArray handling#322
Open
sdiebolt wants to merge 41 commits into
Open
Conversation
…array Resolves #149 and #231. - validate_fusi_dataarray now requires the full z,y,x spatial trio; single-slice data is 3D with a singleton axis (which axis is singleton is unconstrained). Removes the minimum_spatial_dims parameter. Callers (registration, resampling, motion, IQ, smoothing) migrated accordingly. - Add create_fusi_dataarray (confusius.create_fusi_dataarray) building canonical fUSI DataArrays from a raw array plus dt/dz/dy/dx and axis origins, with units/voxdim coordinate metadata and time acquisition-window metadata (volume_acquisition_reference defaults to "start", volume_acquisition_duration defaults to the time spacing). DataArray-level metadata (e.g. beamforming_sound_velocity, transmit_frequency) passes through attrs. - Remove internal 2D SimpleITK/affine handling from the registration stack (motion, volume, affines): registration is 3D-only and transforms are 4x4; motion parameters use the 3D schema (rot_x/y/z, trans_x/y/z). - Fix register_volumewise default learning_rate (0.01 -> 1.0) so motion correction recovers real inter-frame shifts; napari time-series panel matches. BREAKING CHANGE: fUSI DataArrays must contain z, y, and x. Bare (y, x) and (time, y, x) layouts are rejected; represent single-slice data with a singleton axis. register_volume transforms and motion affines are now always 4x4.
…overs Review follow-ups: - create_fusi_dataarray now rejects an out-of-set volume_acquisition_reference at runtime, and builds the time-coordinate attrs inline instead of aliasing a shared dict. - Move the registration progress-plotter tests to 3D SimpleITK images, removing the last 2D transform construct from the test suite. Also records the implementation status in the design doc.
sdiebolt
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
📖 Doc preview: https://confusius.tools/pr-preview/pr-322/ |
Member
Author
|
@FelipeCybis ready to review! |
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
from
July 16, 2026 01:24
5bb9c80 to
d1b0abc
Compare
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
3 times, most recently
from
July 16, 2026 01:43
17a862c to
ef5f93c
Compare
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
from
July 16, 2026 01:47
ef5f93c to
034721f
Compare
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
from
July 16, 2026 10:45
2e9857a to
19d0b4b
Compare
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
from
July 16, 2026 11:05
19d0b4b to
47a060e
Compare
sdiebolt
force-pushed
the
feat/issue-149-231-zyx
branch
from
July 16, 2026 12:50
a2d504b to
fc97d11
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens ConfUSIus’ canonical fUSI xarray.DataArray model around full 3D spatial geometry (z, y, x always present), while adding a constructor helper (create_fusi_dataarray) to build canonical fUSI DataArrays from raw arrays plus spacing/origin metadata. It also introduces scalar-index recovery at API boundaries via canonicalize_fusi_dataarray / ensure_fusi_dataarray, and migrates registration/resampling/motion codepaths and tests to consistently use 3D (4, 4) affines (including singleton-z recordings).
Changes:
- Added
create_fusi_dataarray(exported viaconfusius.xarrayand top-levelconfusius) with strict spacing/metadata validation and canonical dimension ordering. - Enforced
validate_fusi_dataarray()to require all spatial dims (z,y,x) and added canonicalization utilities to restore scalar-indexed spatial dims as singleton axes. - Updated registration/resampling/motion (and napari UI defaults/tests/docs) to treat “2D” fUSI as singleton-
z3D and to use only 3D(4, 4)transforms.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/gallery/execute.py | Executes gallery notebooks in an isolated temporary working directory. |
| tests/unit/test_xarray/test_create.py | New unit coverage for create_fusi_dataarray behaviors and validation. |
| tests/unit/test_validation/test_fusi.py | Adds tests for canonicalize_fusi_dataarray / ensure_fusi_dataarray and new spatial-dim requirements. |
| tests/unit/test_utils/test_coordinates.py | Adds test coverage for singleton-dim spacing/voxdim requirements in grid helpers. |
| tests/unit/test_spatial/test_smooth.py | Updates smoothing tests for new scalar-index behaviors and stricter type handling. |
| tests/unit/test_registration/test_volumewise.py | Migrates volumewise registration tests to singleton-z 3D and (4, 4) affines. |
| tests/unit/test_registration/test_volume.py | Migrates volume registration/resampling tests to singleton-z 3D and (4, 4) affines; adds spacing validation tests. |
| tests/unit/test_registration/test_progress.py | Updates progress-plotter tests to use 3D SimpleITK images/transforms. |
| tests/unit/test_registration/test_motion.py | Removes 2D-affine motion path tests; enforces (4, 4)-only motion computations. |
| tests/unit/test_registration/conftest.py | Updates fixtures to represent single-slice fUSI as singleton-z 3D with voxdim. |
| tests/unit/test_napari/test_registration_progress.py | Updates napari registration progress tests to singleton-z 3D inputs. |
| tests/unit/test_napari/test_registration_panel.py | Updates napari panel defaults and transform application tests for 3D affines/volumes. |
| tests/unit/test_iq/test_process.py | Aligns IQ processing validation error expectations with new spatial-dim requirement text. |
| src/confusius/xarray/create.py | Implements create_fusi_dataarray + helpers for coords/spacings/origins and canonicalization. |
| src/confusius/xarray/init.py | Exposes create_fusi_dataarray via lazy import machinery. |
| src/confusius/validation/iq.py | Removes obsolete minimum_spatial_dims parameter usage (now full spatial trio required). |
| src/confusius/validation/fusi.py | Requires full (z, y, x) presence; adds canonicalize/ensure helpers for scalar-index recovery. |
| src/confusius/validation/init.py | Re-exports canonicalize_fusi_dataarray and ensure_fusi_dataarray. |
| src/confusius/spatial/smooth.py | Adjusts smoothing input checks (type validation) and relies on coordinate spacing via accessor. |
| src/confusius/registration/volumewise.py | Uses ensure_fusi_dataarray for scalar-index recovery + strict 3D validation in motion correction. |
| src/confusius/registration/volume.py | Enforces 3D-only registration API; returns canonicalized moving/fixed; uses Euler3D/(4,4) affines. |
| src/confusius/registration/resampling.py | Uses ensure_fusi_dataarray and expects (4, 4) transforms for fUSI resampling. |
| src/confusius/registration/motion.py | Enforces (4, 4)-only affine validation and 3D motion parameter extraction/FD computation. |
| src/confusius/registration/bspline.py | Tightens reference validation to 3D; enforces defined spacing for displacement-field conversion. |
| src/confusius/registration/affines.py | Removes 2D SimpleITK transform types from linear-transform conversion surface. |
| src/confusius/registration/_utils.py | Requires defined spatial spacing when converting DataArrays to SimpleITK images (no fallback). |
| src/confusius/connectivity/seed.py | Updates doc examples to show singleton-z canonical fUSI usage. |
| src/confusius/_utils/coordinates.py | Makes grid kwarg construction require defined spacing (singleton dims must supply voxdim). |
| src/confusius/_napari/_registration/_panel_parameters.py | Sets consistent default learning rate value (0.01) for registration UI. |
| src/confusius/init.py | Exposes create_fusi_dataarray at the top-level confusius namespace. |
| docs/user-guide/xarray.md | Documents create_fusi_dataarray and scalar-index recovery expectations for canonical fUSI dims. |
| docs/user-guide/io.md | Updates “Other Systems” docs to use create_fusi_dataarray when wrapping external IQ. |
| docs/user-guide/beamformed-iq.md | Updates IQ structure docs and adds an example flow using create_fusi_dataarray + validation. |
| docs/examples/02_registration/02_volumewise_motion_correction.py | Updates example narrative to reflect conservative default learning rate guidance. |
| docs/changelog.md | Adds changelog entries for the breaking geometry requirement and the new constructor helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
# Conflicts: # docs/user-guide/beamformed-iq.md
create_motion_dataframe canonicalized `reference` separately inside compute_framewise_displacement and passed the raw, un-canonicalized array to _get_motion_parameter_columns, so a scalar-indexed reference (e.g. `.isel(z=0)`) raised KeyError instead of being recovered like other registration APIs. Also restores the explicit time-dimension check in compute_framewise_displacement that the ndim==3 hardcoding had silently dropped. fix(xarray): move changelog entries to the current dev version New entries had landed under the already-released 0.6.0 section instead of 0.7.0.dev0. fix(docs): update stale example filename in prefetch script comment refactor(validation): share the DataArray type check via require_dataarray validate_fusi_dataarray, canonicalize_fusi_dataarray, ensure_fusi_dataarray, and smooth_volume each re-implemented the same isinstance/TypeError check; extracted to confusius/_utils/validation.py. fix(utils): drop unreachable None filter in get_grid_kwargs_from_dataarray docs(spatial): document TypeError raised by smooth_volume test(registration): rename 2D-labeled singleton-z fixtures for clarity
- xarray.md: Title-Case the two new create_fusi_dataarray headings to match the rest of the file; move the "New to Xarray?" tip back next to the Datasets/DataArrays intro instead of after the new subsections. - beamformed-iq.md/io.md: de-duplicate the IQ DataArray creation example (kept only in beamformed-iq.md, io.md now links to it) and reorder beamformed-iq.md so "what valid data looks like" comes before "how to build it yourself" instead of the reverse with no transition. - io.md: give the power-Doppler creation example (Loading Other Formats) a distinct frame rate (2.5 Hz vs. IQ's 500 Hz) and link to the full MAT-file gallery example instead of duplicating it inline. - io.md: fix doubled "For example for a" wording.
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
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.
Summary
Notes
Closes #149
Closes #231
Closes #323