✨ Add wk-* CLI suite for warp pipeline operations - #17
Conversation
Expand warpkit's CLI surface from `medic` + `extract_field_from_maps` to six purpose-built tools, all `wk-` prefixed to avoid collisions with namesakes from FSL/ANTs/AFNI/etc.: * `wk-medic` — full pipeline (renamed from `medic`). * `wk-unwrap-phase` + `wk-compute-fieldmap` — the same pipeline split into reusable stages, addresses #12 (unwrapped phase as a first-class output usable for phase regression, T2* estimation, etc.). * `wk-apply-warp` — resample any 3D/4D image through a displacement map/field, single or per-frame; auto-detects map vs field from intent + shape, with an override flag. * `wk-convert-warp` — interconverts maps <-> fields, ITK/FSL/ANTs/AFNI format conversion, single-frame extraction, and (with `--invert`) inverts the warp along the way (1D inverter for series, full 3D inverter for single frames). Replaces `extract_field_from_maps`. * `wk-compute-jacobian` — per-voxel Jacobian determinant for any warp. Behind the scenes: * Refactored `unwrap_and_compute_field_maps` into reusable `unwrap_phases` + `compute_field_maps` stages; the original wrapper is kept and `medic()` still calls it, so the Python API is unchanged. * Replaced the `Romeo` C++ class facade with free functions in `warpkit_cpp` (`romeo_unwrap3d`, `romeo_unwrap4d`, `romeo_voxelquality`, `calculate_weights`); pyi regenerated via `scripts/regen-stub.sh`. * Fixed a pre-existing bug in `invert_displacement_field` that returned a 5-channel array (`np.pad` was padding the channel axis); padding is now spatial-only and the output is the expected (X, Y, Z, 3). * Added `displacement_field_to_map`, the inverse of `displacement_map_to_field`. * Extracted shared CLI IO helpers (frame splitting, classification, bundle/per-frame write) into a private `warpkit/scripts/_warp_io.py`. CI-relevant: * Removed the script auto-discovery glob in `setup.py`; entry points are now explicit in `[project.scripts]` in `pyproject.toml`. No matrix change. * Dockerfile entrypoint switched from `medic` -> `wk-medic`. * Test count 80 -> 111; coverage 74% -> 90%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR expands warpkit’s command-line surface area to a wk-* tool suite for warp/MEDIC pipeline operations, while refactoring the Python/C++ bindings and internal APIs to support staged workflows (unwrap → fieldmap → apply/convert/jacobian) and modernizing packaging/entrypoints.
Changes:
- Introduces new
wk-*CLIs (wk-unwrap-phase,wk-compute-fieldmap,wk-apply-warp,wk-convert-warp,wk-compute-jacobian) and updates docs/Docker to usewk-medic. - Refactors MEDIC unwrap/fieldmap logic into reusable stages (
unwrap_phases,compute_field_maps) and adds new warp utility conversion (displacement_field_to_map) plus a bugfix ininvert_displacement_field. - Replaces the ROMEO pybind class wrapper with module-level free functions and expands tests to include CLI happy paths.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| warpkit/warpkit_cpp.pyi | Updates type stubs for ROMEO bindings refactor (free functions, no Romeo class). |
| warpkit/utilities.py | Adds displacement_field_to_map and fixes spatial-only padding in invert_displacement_field. |
| warpkit/unwrap.py | Switches ROMEO usage from class instance to module-level binding functions; adds staged unwrap/fieldmap APIs. |
| warpkit/scripts/unwrap_phase.py | New CLI for phase unwrapping stage outputting per-echo unwrapped phase + masks. |
| warpkit/scripts/medic.py | Updates CLI metadata/epilog formatting (wk-prefixed usage in tests/docs). |
| warpkit/scripts/extract_field_from_maps.py | Removes legacy CLI superseded by wk-convert-warp. |
| warpkit/scripts/convert_warp.py | New CLI for map/field conversion, format conversion, and inversion with shared IO helpers. |
| warpkit/scripts/compute_jacobian.py | New CLI to compute Jacobian determinant per frame from map/field inputs. |
| warpkit/scripts/compute_fieldmap.py | New CLI for post-unwrap fieldmap/displacement-map computation mirroring MEDIC stage 2. |
| warpkit/scripts/apply_warp.py | New CLI to resample images through map/field transforms (single or per-frame). |
| warpkit/scripts/_warp_io.py | New shared IO: input classification/splitting and bundled/per-frame output writing. |
| tests/test_utilities.py | Updates inversion shape expectations and adds Jacobian determinant zero-field test. |
| tests/test_unwrap.py | Updates ROMEO binding smoke tests for free-function API. |
| tests/test_scripts.py | Expands CLI coverage: validation + happy-path execution for the new wk-* scripts. |
| tests/test_romeo.py | Updates ROMEO tests to use free-function bindings (no Romeo fixture). |
| tests/conftest.py | Adds fixture returning test-data file paths for CLI tests. |
| src/warpkit.cpp | Updates pybind11 module exports: removes Romeo class, registers ROMEO free functions. |
| setup.py | Removes console-script auto-discovery (entrypoints now owned by pyproject.toml). |
| pyproject.toml | Declares explicit [project.scripts] for wk-* CLI entry points (no longer dynamic). |
| include/romeo/romeo.h | Refactors ROMEO Python-facing API from a facade class to free functions. |
| README.md | Documents wk-* CLIs and adds follow-on workflow examples. |
| Dockerfile | Switches container entrypoint from medic to wk-medic. |
| CLAUDE.md | Updates contributor/CI guidance to match explicit wk-* scripts and CI/coverage workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
===========================================
+ Coverage 83.54% 94.44% +10.90%
===========================================
Files 9 15 +6
Lines 632 1081 +449
===========================================
+ Hits 528 1021 +493
+ Misses 104 60 -44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds a seventh wk-* CLI: convert between mm displacement maps/fields and Hz B0 field maps in either direction, using the same per-frame input model as `wk-convert-warp`. * Auto-classifies 1-channel inputs by inference from `--to`: when going to a fieldmap the input is mm displacement; when going to a map/field the input is a Hz fieldmap. * On the mm side, accepts either 1-channel maps or 3-channel fields; field inputs have the PE-axis channel extracted before unit conversion, and field outputs are promoted from the mm map back to 3 channels. * `--total-readout-time` and `--phase-encoding-direction` are required (the conversion is `displacement = fieldmap * trt * voxel_size_along_PE`); `--flip-sign` mirrors the flip-sign branch in `warpkit.distortion.medic`. * Refuses mm <-> mm conversions and same-`--from`-as-`--to` calls, pointing users at `wk-convert-warp` for those cases. Also: * Reuses `_warp_io.read_input_frames` and `write_output` for input splitting and bundle/split output (no new IO code). * 9 new tests cover help, all the validation paths, the mm-Hz-mm round-trip (preserved within 1e-5), and the field-input axis extraction. Total tests 111 -> 120; coverage holds at 90%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 5D branch had an inverted condition: it raised when the input had the canonical ANTs/AFNI single-warp layout (X, Y, Z, 1, 3) — exactly the shape the error message described as required. Rewrite the validation to accept 4D (X, Y, Z, 3) and 5D (X, Y, Z, 1, 3) and reject everything else. The CLIs masked this because read_input_frames splits 5D files into 4D frames upstream, so it only fired on direct Python-API use — including the documented displacement_map_to_field -> displacement_field_to_map roundtrip for the ants/afni formats, which produce a 5D intermediate. Add regression tests covering 5D-accepting, 5D-rejecting, and the full (axis, format) roundtrip matrix for the map<->field pair. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… validation Three intent-leak fixes — a 1-channel map output that inherits a vector intent from an upstream field operation gets re-classified as a field by the auto-classifier, leading to surprising behavior: * displacement_field_to_map copies the intermediate field header verbatim. Copy and reset to "none" so the scalar map is classified as a map. * bundle_frames_to_3d_series stacks scalar frames but reuses the first frame's header. Same fix. * write_output's per-frame branch wrote each frame untouched. For out_type=="map", clear the intent before writing. wk-unwrap-phase now checks --magnitude and --metadata counts match --phase up front, so users get a clean argparse error instead of a downstream zip(strict=True) ValueError. The count check was moved before the JSON sidecar load so a count mismatch isn't masked by a FileNotFoundError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-detection of map vs. field from NIfTI shape + intent was a hidden
source of surprise — a stale vector intent code (or a 4D shape with
last==3) would silently flip a map to a field. The user is now required
to declare the input type up front:
* wk-convert-warp / wk-convert-fieldmap / wk-compute-jacobian: --from is
required, choices are {map, field} (plus 'fieldmap' for convert-fieldmap).
* wk-apply-warp: --transform-type is required, {map, field}. Multi-file
--transform now requires --transform-type=field (was implicit).
* _warp_io.classify is gone (only ever served the auto path); the
shape-mismatch check inside read_input_frames remains as the user's
declared type can still be inconsistent with the actual file.
* convert_fieldmap._resolve_in_type collapses to a no-op now that --from
is always explicit; the consistency check moved into the shape-split
branch of read_input_frames.
Test churn is mechanical (every CLI invocation gains an explicit --from
or --transform-type). The mixed-input-type test for convert_warp is
removed since one --from value now applies to all inputs by construction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes coverage gaps left by the auto-removal cleanup. The mm<->Hz path had only roundtrip and flip-sign coverage; format / orientation / inversion paths only had zero-input or roundtrip coverage. New tests target absolute values, sign conventions, and CLI-level behavior. In tests/test_utilities.py: * Hand-computed fmap -> dmap value, parametrized over all 6 PE codes (locks the LPS-x/y vs z sign convention). * Anisotropic-voxel scaling (verifies the right axis's voxel is used). * PE-sign symmetry: '<axis>' and '<axis>-' produce exact negations. * displacement_map_to_field frame=N selection uses the right frame. * displacement_field_to_map drops off-axis channels (asserted directly). * convert_warp itk->itk roundtrip on an LPS affine (covers the as_reoriented round trip). * compute_jacobian_determinant of a constant translation = 1 (the non-zero counterpart to the existing zero-field test). In tests/test_scripts.py: * wk-convert-warp --frame extracts the right frame; per-frame outputs preserve frame ordering. * wk-convert-fieldmap --frame extracts the right frame, with a hand-computed expected Hz->mm value as a sanity check. * wk-convert-fieldmap with vs without --flip-sign produces exact negations. * 5D ANTs single-warp file roundtrip through wk-convert-warp via files on disk (regression for the 5D shape-validation fix). * CLI Hz->mm->invert->Hz(--flip-sign) chain reproduces the medic non-native fieldmap output bit-for-bit. * invert(invert(map)) approximately recovers the original on a small smooth displacement (loose tolerance, interior only). 173 tests pass total (33 added). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds tests for the previously-uncovered branches across all six CLIs plus a few low-level utility paths. CLI branches covered: * wk-apply-warp: 3D map transform, 5D ANTs single-warp, 5D field series, --reference grid, --transform-type=map with multi-file --transform rejection, --transform-type=field with non-itk --format, invalid field/input/map shape errors. * wk-compute-jacobian: ANTs-format input, --frame selection, bundled vs. per-frame outputs are bit-identical. * wk-convert-warp: --invert + format conversion combo, multi-frame field --invert routing (to default field output AND --to=map output). * wk-convert-fieldmap: multi-file fieldmap input, 5D field input, --frame out-of-range error. * wk-unwrap-phase: --noiseframes strips trailing volumes (asserts shape reduction on the bundled fixture). Utility branches covered: * convert_warp out_type=bogus rejected (separate from in_type branch). * resample_image rejects wrong-channel transform; squeezes 5D transform. * create_brain_mask: positive extra_dilation grows mask, negative erodes. * bundle_frames_to_field_series squeezes 5D singleton inputs. Coverage on warpkit/scripts + warpkit/utilities went from 94% to 99% (201 tests, was 173). Remaining 8 lines are essentially-unreachable defensive raises and one debug branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
warpkit/unwrap.py:741
unwrap_phases()converts 3D inputs to a single-frame 4D series, but it only forcesframes=[0]whenframes is None. If a caller passesframes=[1](or any non-zero index) with 3D inputs,phase_iterator()will indexdataobj[..., frame_idx]out of range and crash. Consider validating that all requested frame indices are within[0, phase[0].shape[-1])after the 3D→4D promotion (and either raise a clearValueErroror coerceframesto[0]).
# check if data is 4D or 3D
if len(phase[0].shape) == 3:
# convert data to 4D
phase = [
nib.Nifti1Image(p.get_fdata()[..., np.newaxis], p.affine, p.header)
for p in phase
]
mag = [
nib.Nifti1Image(m.get_fdata()[..., np.newaxis], m.affine, m.header)
for m in mag
]
if frames is None:
frames = [0]
elif len(phase[0].shape) == 4:
# if frames is None, set it to all frames
if frames is None:
frames = list(range(phase[0].shape[-1]))
else:
raise ValueError("Data must be 3D or 4D.")
# frames should be a list at this point
frames = cast(list[int], frames)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…lp text - compute_field_maps now validates masks shape against unwrapped data (raises ValueError instead of misbehaving deeper in SVD). - wk-unwrap-phase rejects negative --noiseframes and any value that would leave 0 frames per input, with a clean parser.error. - wk-convert-warp --invert help now describes the per-frame routing and the off-axis-component drop for multi-frame field input. - wk-convert-warp --to help no longer claims auto-classification (we dropped --from=auto in 9164bcb); now says it defaults to --from. - New regression tests for each. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Expand warpkit's CLI from
medic+extract_field_from_mapsto sevenpurpose-built tools, all
wk-prefixed to avoid colliding with namesakesfrom FSL/ANTs/AFNI/etc.:
wk-medicmedic).wk-unwrap-phasewk-compute-fieldmapwk-apply-warpwk-convert-warp--invert.wk-convert-fieldmapwk-compute-jacobianCloses #12 —
wk-unwrap-phaseexposes unwrapped phase as a first-classoutput usable outside the distortion-correction pipeline (phase regression,
T2* estimation).
Changes
New CLIs / refactor:
unwrap_and_compute_field_mapssplit into reusableunwrap_phases+compute_field_mapsstages; the wrapper is kept somedic()'s PythonAPI is unchanged.
--invertonwk-convert-warproutes by frame count:N==1uses thefull 3D
invert_displacement_field;N>1runsinvert_displacement_mapsonce on the stacked 4D series (faster per frame, requires
--axis).wk-convert-warpandwk-compute-jacobianrequire--from,wk-apply-warprequires--transform-type. Auto-classification was prototyped and then dropped(9164bcb) — silently misclassifying a 1-channel map as a 3-channel
field (or vice versa) on a stripped intent code is worse than a clean
argparse error.
Bug fix surfaced while wiring up
wk-convert-warp --invert:invert_displacement_fieldwas usingnp.pad(data, pad_width=1)whichpadded the channel axis as well as the spatial axes; the output array
came back with last-dim = 5 instead of 3. Now padding is spatial-only
(
((1,1), (1,1), (1,1), (0,0))). The existing test that documentedthis as "out of scope" is updated to assert the full shape match.
Bindings:
RomeoC++ class facade replaced with free functions inwarpkit_cpp(
romeo_unwrap3d,romeo_unwrap4d,romeo_voxelquality,calculate_weights).warpkit_cpp.pyiregenerated viascripts/regen-stub.sh.Internal API:
displacement_field_to_map(inverse ofdisplacement_map_to_field).intent normalization for scalar outputs) extracted into a private
warpkit/scripts/_warp_io.py.Tests + coverage:
completion against the bundled MEDIC fixture (or tiny synthetic NIfTIs)
and assert on the output files. Previous suite was argparse-only.
test_compute_jacobian_determinant_zero_field(zero field → J=1).CI-relevant
setup.pyno longer auto-discoverswarpkit/scripts/*.py; entrypoints are listed explicitly in
[project.scripts]inpyproject.toml.No wheel-matrix change.
ENTRYPOINTswitched frommedic→wk-medic."Common follow-on workflows" with concrete examples for
wk-apply-warp,wk-convert-warp,wk-convert-fieldmap,wk-compute-jacobian).cp314t-*, not alsocp313t-*), addedubuntu-24.04-armto the runnerlist, and added a
coveragesnippet to the dev workflow.Test plan
uv run pytest -q— 206/206 passing locallyuv run coverage run && uv run coverage report— 95% total coverageuv run pre-commit run --all-files— ruff/format/pyright cleandocker run ... --help)🤖 Generated with Claude Code