✨ Expose typed Python API for the seven wk-* operations#21
Conversation
There was a problem hiding this comment.
Pull request overview
This PR exposes a typed, programmatic Python API for the existing seven wk-* CLI operations by adding keyword-only entry points that raise ValueError (instead of SystemExit) and return frozen result dataclasses, while keeping CLI behavior stable via thin main() shims.
Changes:
- Added typed entry-point functions + frozen
Resultdataclasses to script modules, withmain()forwardingValueErrortoparser.error. - Introduced shared helpers in
warpkit/scripts/_metadata.pyand refactored_warp_io.pyto validate viaValueErrorand accept in-memoryNifti1Imageinputs. - Added
warpkit/api.pyas a single re-export surface for library integrators; updated one test for the newwrite_outputsignature.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| warpkit/scripts/unwrap_phase.py | Adds typed unwrap_phase() + result dataclass; CLI becomes a shim catching ValueError. |
| warpkit/scripts/medic.py | Adds typed medic() + result dataclass; CLI shim; refactors call into distortion pipeline. |
| warpkit/scripts/convert_warp.py | Adds typed convert_warp() + result dataclass; CLI shim; avoids naming clash with utility converter. |
| warpkit/scripts/convert_fieldmap.py | Adds typed convert_fieldmap() + result dataclass; CLI shim with message mapping. |
| warpkit/scripts/compute_jacobian.py | Adds typed compute_jacobian() + result dataclass; CLI shim. |
| warpkit/scripts/compute_fieldmap.py | Adds typed compute_fieldmap() + result dataclass; CLI shim; centralizes acquisition resolution. |
| warpkit/scripts/apply_warp.py | Adds typed apply_warp() + result dataclass; refactors transform validation to raise ValueError. |
| warpkit/scripts/_warp_io.py | Refactors shared warp I/O to raise ValueError (no argparse dependency) and accept in-memory images. |
| warpkit/scripts/_metadata.py | New shared helpers for image coercion, acquisition metadata resolution, and noise-frame trimming. |
| warpkit/api.py | New re-export module for the typed API surface. |
| tests/test_scripts.py | Updates write_output() call site to match new signature. |
💡 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 #21 +/- ##
==========================================
- Coverage 95.42% 94.71% -0.71%
==========================================
Files 15 16 +1
Lines 1071 1249 +178
==========================================
+ Hits 1022 1183 +161
- Misses 49 66 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Each warpkit.scripts.<name> module now exports a typed function (``medic``, ``unwrap_phase``, ``compute_fieldmap``, ``apply_warp``, ``convert_warp``, ``convert_fieldmap``, ``compute_jacobian``) and a frozen Result dataclass alongside the existing ``main()`` argparse entry point. ``main()`` is now a thin shim: parse argv → call the typed function → forward ``ValueError`` to ``parser.error`` so existing CLI behaviour (``SystemExit(2)``, ``error: ...`` on stderr) is preserved. A new top-level ``warpkit.api`` module re-exports all seven functions + result types for convenient ``from warpkit.api import medic`` usage by library integrators (e.g. nipype interfaces, fmriprep). - New ``warpkit/scripts/_metadata.py`` centralises BIDS sidecar loading, the either-or/mutex acquisition resolution, image coercion, and noise-frame trimming previously duplicated across three scripts. - ``warpkit/scripts/_warp_io.py`` no longer takes an ``argparse.ArgumentParser``; ``read_input_frames`` / ``write_output`` raise ``ValueError`` and the CLI shims forward to ``parser.error``. ``read_input_frames`` also accepts already-loaded ``Nifti1Image`` objects in addition to paths. - All seven typed functions are keyword-only and accept either paths or ``nib.Nifti1Image`` for image inputs; outputs are written and returned as absolute ``pathlib.Path`` in the result dataclass. No CLI behaviour changes; all 207 existing tests pass.
15929b1 to
ec0cf74
Compare
Default Codecov policy treats any project-coverage decrease as a failure and requires patch coverage to match the base. New CLI shims add unavoidable argparse / __main__ glue, so a small drop is expected. Allow 1% slack on project and pin patch to a 90% floor (with 1% slack) instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review feedback on the typed Python API: - ``trim_noise_frames`` now rejects 3D inputs (``[..., :-n]`` would chop the Z dimension instead of frames) and ``n >= n_frames`` (which silently yielded an empty 4D series). Both now raise ``ValueError`` from the helper itself, so ``medic()`` and any future caller inherit the same safety; the duplicated guard inside ``unwrap_phase()`` is dropped. - ``load_acquisition_from_metadata`` honours ``require_trt_pe``: callers that only need ``EchoTime`` (e.g. ``unwrap_phase``) accept sidecars without ``TotalReadoutTime`` / ``PhaseEncodingDirection``. Missing keys surface as a clean ``ValueError`` instead of a ``KeyError``. Adds CLI tests for the EchoTime-only sidecar path, the missing-EchoTime error, and ``-f >= n_frames`` rejection in ``wk-medic``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
warpkit.scripts.<name>module now exports a typed function (medic,unwrap_phase,compute_fieldmap,apply_warp,convert_warp,convert_fieldmap,compute_jacobian) and a frozenResultdataclass alongside the existingmain()argparse entry point.warpkit.apimodule re-exports all seven functions + result types forfrom warpkit.api import medicusage by library integrators (nipype, fmriprep, etc.).main()is a thin shim: parse argv → call the typed function → forwardValueErrortoparser.error. CLI behaviour is preserved exactly (SystemExit(2), same stderr text).Why
To support library integration (nipype interface bindings being written next), we need a typed Python entry point per CLI tool that accepts kwargs, raises
ValueError, and returns a result dataclass — instead of forcing callers to fabricateargvand catchSystemExit.Changes
warpkit/api.py— new public re-export module.warpkit/scripts/_metadata.py— new; centralises BIDS sidecar resolution, the either-or/mutex acquisition validation, image coercion (Path | str | Nifti1Image), and noise-frame trimming previously duplicated across three scripts.warpkit/scripts/_warp_io.py— no longer takes anargparse.ArgumentParser;read_input_frames/write_outputraiseValueErrorand the CLI shims forward toparser.error.read_input_framesalso accepts already-loadedNifti1Imageobjects in addition to paths.warpkit/scripts/{medic,unwrap_phase,compute_fieldmap,apply_warp,convert_warp,convert_fieldmap,compute_jacobian}.py— each gains a typed function + Result dataclass;main()shrinks to ~argparse +try/except ValueError → parser.error.test_write_output_per_frame_map_clears_vector_intent) updated to match the newwrite_outputsignature.API shape
All seven functions are keyword-only and accept paths or
nib.Nifti1Imagefor image inputs:Test plan
uv run pytest -q— 207/207 passeduvx ruff check warpkit/— cleanuvx ruff format --check warpkit/— cleanuvx pyright warpkit/— clean (only the pre-existingwarpkit_cppstub-resolution warning)parser.errortext/SystemExit semantics preserved