ENH: Update ConvertImage4DTo3D to support vector 3D and 4D#51
Conversation
Use ITK reader by default, except for NRRD which stores 4D data as images of vectors which is not available in ITK's python wrapping.
WalkthroughThis PR replaces the NRRD-specific 4D-to-3D converter with a generalized Changes4D Image Conversion Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Renames ConvertNRRD4DTo3D → ConvertImage4DTo3D and broadens it to use ITK readers by default for 4D inputs (NIfTI, MHA, …), keeping the pynrrd path only for .nrrd/.seq.nrrd files where ITK Python lacks the needed wrapped vector pixel sizes. Tests, workflows, experiments, docs and CI references are updated to follow the rename.
Changes:
- New
convert_image_4d_to_3d.pywith format-dispatched loader (NRRD → pynrrd, others →itk.imread) and 3D origin/spacing/direction extraction. - Old
convert_nrrd_4d_to_3d.pyremoved; workflow now reads multi-file 3D inputs directly withitk.imreadinstead of viaload_nrrd_3d. - Package re-exports, CLI entry point, tests, experiments, and documentation renamed/updated.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/physiomotion4d/convert_image_4d_to_3d.py | New ITK-based 4D→3D converter (replaces the NRRD-only one). |
| src/physiomotion4d/convert_nrrd_4d_to_3d.py | Old module removed. |
| src/physiomotion4d/init.py | Re-exports ConvertImage4DTo3D instead of ConvertNRRD4DTo3D. |
| src/physiomotion4d/cli/init.py | Adds convert_image_4d_to_3d to __all__ (module file appears missing). |
| src/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py | Uses new converter; multi-file path now uses itk.imread directly. |
| pyproject.toml | New physiomotion4d-convert-image-4d-to-3d script entry point (target module appears missing). |
| tests/test_convert_image_4d_to_3d.py | Renamed test suite using new class/methods. |
| tests/conftest.py | Fixture updated to new converter API. |
| tests/test_register_images_ants.py, test_register_images_icon.py, test_segment_chest_total_segmentator.py | Docstring updates referencing renamed test. |
| tests/README.md, TESTING_GUIDE.md, GITHUB_WORKFLOWS.md | Doc references updated to new test name. |
| docs/architecture.rst, testing.rst, API_MAP.md, cli_scripts/overview.rst, api/utilities/index.rst, api/utilities/image_conversion.rst | Docs renamed/updated for new module and CLI. |
| experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py, experiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.py, data/Slicer-Heart-CT/download_and_convert.py | Example scripts updated to new API. |
| .github/workflows/ci.yml | CI runs renamed test file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # CLI commands installed via pip | ||
| # Entry points reference the main() functions in the cli submodule | ||
| physiomotion4d-convert-ct-to-vtk = "physiomotion4d.cli.convert_ct_to_vtk:main" | ||
| physiomotion4d-convert-image-4d-to-3d = "physiomotion4d.cli.convert_image_4d_to_3d:main" |
|
|
||
| __all__ = [ | ||
| "convert_heart_gated_ct_to_usd", | ||
| "convert_image_4d_to_3d", |
| def load_image_4d(self, filename: str) -> None: | ||
| """Load a 4D image and split it into a list of 3D ITK images. | ||
|
|
||
| ``.nrrd`` files (including Slicer ``.seq.nrrd`` heart sequences) are | ||
| read with ``pynrrd`` because their per-voxel vector dimension exceeds | ||
| the component counts wrapped in ITK Python. Every other format is | ||
| read with ``itk.imread`` and must already be a 4-dimensional image. | ||
|
|
||
| The result is a (T, Z, Y, X) ndarray plus 3D origin / spacing / | ||
| direction; each temporal slice becomes a standalone 3D ITK image in | ||
| ``self.img_3d``. | ||
|
|
||
| Args: | ||
| filename: Path to a 4D image file. | ||
| """ | ||
| if filename.lower().endswith(".nrrd"): |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_convert_image_4d_to_3d.py (1)
43-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
test_slice_files_createdself-contained to avoid order-dependent failures.Line 52 only inspects existing files and never performs conversion, so this test can fail when run alone (or before
test_convert_4d_to_3d). Generate slices in this test (or a fixture) before asserting counts.Proposed fix
def test_slice_files_created( self, download_test_data: Path, test_directories: dict[str, Path], ) -> None: """Test that all expected slice files are present after conversion.""" output_dir = test_directories["output"] / "convert_image_4d_to_3d" output_dir.mkdir(parents=True, exist_ok=True) + conv = ConvertImage4DTo3D() + conv.load_image_4d(str(download_test_data)) + conv.save_3d_images(output_dir, "slice") + expected_count = conv.get_number_of_3d_images() + slice_files = list(output_dir.glob("slice_*.mha")) - assert len(slice_files) > 10, ( - f"Expected more than 10 slice files, found {len(slice_files)}" - ) + assert len(slice_files) == expected_count, ( + f"Expected {expected_count} slice files, found {len(slice_files)}" + ) slice_007 = output_dir / "slice_007.mha" assert slice_007.exists(), "Expected slice_007.mha not found"🤖 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/test_convert_image_4d_to_3d.py` around lines 43 - 60, The test_slice_files_created test inspects output_dir but never runs the conversion, causing order-dependent failures; modify the test to first invoke the conversion step (call the conversion function or CLI entrypoint used elsewhere, e.g., the same routine exercised by test_convert_4d_to_3d) using download_test_data as input and output_dir as target so slices are generated in this test, then assert on list(output_dir.glob("slice_*.mha")) and slice_007.mha existence; keep the same output_dir creation (output_dir.mkdir(...)) and assertions but add the conversion call at the start of test_slice_files_created to make it self-contained.
🧹 Nitpick comments (3)
src/physiomotion4d/convert_image_4d_to_3d.py (3)
29-29: ⚡ Quick winUse
Union[int, str]for consistency.The coding guidelines specify
Optional[X]instead ofX | Noneto maintain compatibility. For consistency, union types should useUnion[int, str]rather thanint | str. As per coding guidelines, "UseOptional[X]instead ofX | None(ruffUP007suppressed)".♻️ Proposed fix
- def __init__(self, log_level: int | str = logging.INFO) -> None: + def __init__(self, log_level: Union[int, str] = logging.INFO) -> None:🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` at line 29, The __init__ annotation should use typing.Union for union types to match project conventions: change the signature of the constructor method __init__ in convert_image_4d_to_3d.py from "log_level: int | str" to "log_level: Union[int, str]" and add the corresponding "from typing import Union" import (or include Union in an existing typing import) so the type hint is consistent with the codebase guidelines.
97-99: ⚡ Quick winUse specific return type instead of
Any.The method returns an ITK image from
self.img_3d[index], which storesitk.Imageobjects (line 91). The return type should beitk.Imageinstead ofAnyfor type safety.♻️ Proposed fix
- def get_3d_image(self, index: int) -> Any: + def get_3d_image(self, index: int) -> itk.Image: """Return the 3D ITK image at the given time index.""" return self.img_3d[index]🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` around lines 97 - 99, The get_3d_image method currently declares a return type of Any but always returns an ITK image from self.img_3d; change the signature of get_3d_image to return itk.Image (or the specific itk.Image[PixelType, 3] if you know the pixel type) instead of Any and ensure the module imports itk so the type is available; update any type hints/usages that rely on the old Any to the new itk.Image return type (reference: method get_3d_image and attribute self.img_3d).
36-36: ⚡ Quick winReplace
Anywithitk.Imagefor type safety.The list stores ITK images (line 91 appends
itk.image_from_array(...)results), so the type should belist[itk.Image]instead oflist[Any].♻️ Proposed fix
- self.img_3d: list[Any] = [] + self.img_3d: list[itk.Image] = []🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` at line 36, Replace the broad annotation list[Any] on the instance variable self.img_3d with the specific itk image type by changing it to list[itk.Image]; ensure the module imports or references itk so the type is resolvable (e.g., import itk at top) and remove or adjust any unused Any imports—this narrows the type to the actual objects appended in convert_image_4d_to_3d (where itk.image_from_array(...) results are stored).
🤖 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 `@docs/api/utilities/image_conversion.rst`:
- Around line 7-8: The overview sentence currently states that all formats use
ITK readers; update it to clarify that while most 4D medical image formats
(NRRD, NIfTI, MHA, …) are read using ITK readers, NRRD may fall back to the
pynrrd reader for vector-valued data. Edit the top-line text in
image_conversion.rst (the Utilities for converting ... sentence) to explicitly
mention the NRRD/pynrrd exception so users aren’t misled.
In `@docs/cli_scripts/overview.rst`:
- Line 57: Update the sentence "Split a 4D medical image into a 3D time series
using ITK readers" to mention the NRRD fallback for vector-valued cases, e.g.,
rephrase to say it uses ITK readers where possible and falls back to NRRD
parsing for vector-valued images; ensure the new wording clearly communicates
both the primary ITK path and the NRRD exception for vector-valued data so
readers understand the fallback behavior.
In `@src/physiomotion4d/convert_image_4d_to_3d.py`:
- Around line 61-74: The code assumes required NRRD header fields exist when
building spacing_3d and direction_3d; add explicit validation around header
before using header["space origin"], header["space directions"], and
header["measurement frame"] in the function that computes
spacing_3d/direction_3d (the block that assigns spacing_3d, direction_3d and
inspects space). Check that header contains keys "space origin", "space
directions" (and that it has at least 4 rows/columns as used by header["space
directions"][x+1][x]) and "measurement frame" (and that its shape matches 3
elements); if any are missing or malformed raise a clear ValueError (or catch
and re-raise with a descriptive message) indicating the file is not a valid
Slicer .seq.nrrd, so callers of load_image_4d() get a user-friendly error
instead of a KeyError.
---
Outside diff comments:
In `@tests/test_convert_image_4d_to_3d.py`:
- Around line 43-60: The test_slice_files_created test inspects output_dir but
never runs the conversion, causing order-dependent failures; modify the test to
first invoke the conversion step (call the conversion function or CLI entrypoint
used elsewhere, e.g., the same routine exercised by test_convert_4d_to_3d) using
download_test_data as input and output_dir as target so slices are generated in
this test, then assert on list(output_dir.glob("slice_*.mha")) and slice_007.mha
existence; keep the same output_dir creation (output_dir.mkdir(...)) and
assertions but add the conversion call at the start of test_slice_files_created
to make it self-contained.
---
Nitpick comments:
In `@src/physiomotion4d/convert_image_4d_to_3d.py`:
- Line 29: The __init__ annotation should use typing.Union for union types to
match project conventions: change the signature of the constructor method
__init__ in convert_image_4d_to_3d.py from "log_level: int | str" to "log_level:
Union[int, str]" and add the corresponding "from typing import Union" import (or
include Union in an existing typing import) so the type hint is consistent with
the codebase guidelines.
- Around line 97-99: The get_3d_image method currently declares a return type of
Any but always returns an ITK image from self.img_3d; change the signature of
get_3d_image to return itk.Image (or the specific itk.Image[PixelType, 3] if you
know the pixel type) instead of Any and ensure the module imports itk so the
type is available; update any type hints/usages that rely on the old Any to the
new itk.Image return type (reference: method get_3d_image and attribute
self.img_3d).
- Line 36: Replace the broad annotation list[Any] on the instance variable
self.img_3d with the specific itk image type by changing it to list[itk.Image];
ensure the module imports or references itk so the type is resolvable (e.g.,
import itk at top) and remove or adjust any unused Any imports—this narrows the
type to the actual objects appended in convert_image_4d_to_3d (where
itk.image_from_array(...) results are stored).
🪄 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: 352186cf-03a7-40e9-99c0-fce8b67b2984
📒 Files selected for processing (24)
.github/workflows/ci.ymldata/Slicer-Heart-CT/download_and_convert.pydocs/API_MAP.mddocs/api/utilities/image_conversion.rstdocs/api/utilities/index.rstdocs/architecture.rstdocs/cli_scripts/overview.rstdocs/testing.rstexperiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.pypyproject.tomlsrc/physiomotion4d/__init__.pysrc/physiomotion4d/cli/__init__.pysrc/physiomotion4d/convert_image_4d_to_3d.pysrc/physiomotion4d/convert_nrrd_4d_to_3d.pysrc/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.pytests/GITHUB_WORKFLOWS.mdtests/README.mdtests/TESTING_GUIDE.mdtests/conftest.pytests/test_convert_image_4d_to_3d.pytests/test_register_images_ants.pytests/test_register_images_icon.pytests/test_segment_chest_total_segmentator.py
💤 Files with no reviewable changes (1)
- src/physiomotion4d/convert_nrrd_4d_to_3d.py
| * - ``physiomotion4d-convert-ct-to-vtk`` | ||
| - Segment one CT image and export anatomy-group VTK surfaces and meshes | ||
| * - ``physiomotion4d-convert-image-4d-to-3d`` | ||
| - Split a 4D medical image into a 3D time series using ITK readers |
There was a problem hiding this comment.
Broaden description to include the NRRD fallback path.
The description currently states ITK readers only; this misses the NRRD exception path for vector-valued cases.
Suggested wording
- - Split a 4D medical image into a 3D time series using ITK readers
+ - Split a 4D medical image into a 3D time series (ITK by default, with NRRD-specific fallback handling)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Split a 4D medical image into a 3D time series using ITK readers | |
| - Split a 4D medical image into a 3D time series (ITK by default, with NRRD-specific fallback handling) |
🤖 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 `@docs/cli_scripts/overview.rst` at line 57, Update the sentence "Split a 4D
medical image into a 3D time series using ITK readers" to mention the NRRD
fallback for vector-valued cases, e.g., rephrase to say it uses ITK readers
where possible and falls back to NRRD parsing for vector-valued images; ensure
the new wording clearly communicates both the primary ITK path and the NRRD
exception for vector-valued data so readers understand the fallback behavior.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
==========================================
+ Coverage 29.64% 30.09% +0.45%
==========================================
Files 48 49 +1
Lines 6611 6728 +117
==========================================
+ Hits 1960 2025 +65
- Misses 4651 4703 +52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_convert_image_4d_to_3d.py (1)
25-25: ⚡ Quick winAdd shape/axis-order details to test docstrings.
Please include explicit shape and axis order (for example
shape (X, Y, Z, T)) in these test docstrings for contract clarity.As per coding guidelines, "State image shape and axis order in every test docstring: e.g.
shape (X, Y, Z, T)".Also applies to: 48-48, 67-67, 82-82
🤖 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/test_convert_image_4d_to_3d.py` at line 25, Update the test docstrings in tests/test_convert_image_4d_to_3d.py to state the image shape and axis order explicitly (e.g. "shape (X, Y, Z, T) where T is time"); replace the generic docstring """Test conversion of 4D image to 3D time series.""" with a more specific one like """Test conversion of 4D image to 3D time series (input shape (X, Y, Z, T), axes = X,Y,Z,T).""" Do this for the occurrences at the current docstrings (lines shown in the review: 25, 48, 67, 82) so each test clearly documents the expected input shape and axis ordering.
🤖 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 `@src/physiomotion4d/cli/convert_image_4d_to_3d.py`:
- Around line 4-7: The module docstring in convert_image_4d_to_3d.py claims the
CLI accepts an ordered list of 3D image filenames but the CLI only supports a
single 4D input via the --input-image option; either remove the "ordered list of
3D image filenames" claim from the module docstring or implement the missing
mode by extending the argument parser (the function or block that builds
argparse and handles --input-image) to accept multiple inputs (e.g., make
--input-image nargs='+' or add a separate --input-images argument), update the
main handler that currently expects a single 4D image to branch on the new
multi-input case and republish files as "{basename}_{i:03d}.mha", and ensure the
module docstring and any public function docstrings reflect the final behavior.
In `@src/physiomotion4d/convert_image_4d_to_3d.py`:
- Around line 68-80: The code assumes header["space directions"] and
header["measurement frame"] are 2-D arrays but np.asarray may produce 1-D object
arrays (shape (4,) or (3,)) causing IndexError; update the parsing in
convert_image_4d_to_3d.py to coerce and validate shapes before any 2-D indexing:
after space_directions = np.asarray(header["space directions"]) and
measurement_frame = np.asarray(header["measurement frame"]) ensure both arrays
are at least 2-D (use np.atleast_2d or reshape where appropriate), check ndim
and that space_directions.shape >= (4,3) and measurement_frame.shape >= (3,3)
(columns >=3), and raise a clear ValueError referencing filename if validation
fails; also adjust subsequent indexing sites such as space_directions[x + 1][x]
and measurement_frame[...] to assume validated 2-D shapes.
In `@tests/test_convert_image_4d_to_3d.py`:
- Around line 26-27: The tests share a single output_dir (variable output_dir
built from test_directories["output"] / "convert_image_4d_to_3d") which makes
tests order-dependent; modify each test to create an isolated per-test
subdirectory (e.g., append a unique name derived from the test function name, a
UUID, or use pytest's tmp_path fixture) before calling output_dir.mkdir(...) or
alternatively remove any existing slice_*.mha files in that directory before
assertions; update the three places that construct output_dir (the occurrences
around output_dir = test_directories["output"] / "convert_image_4d_to_3d" and
its siblings) so each test writes to its own fresh folder to avoid stale files
satisfying assertions.
---
Nitpick comments:
In `@tests/test_convert_image_4d_to_3d.py`:
- Line 25: Update the test docstrings in tests/test_convert_image_4d_to_3d.py to
state the image shape and axis order explicitly (e.g. "shape (X, Y, Z, T) where
T is time"); replace the generic docstring """Test conversion of 4D image to 3D
time series.""" with a more specific one like """Test conversion of 4D image to
3D time series (input shape (X, Y, Z, T), axes = X,Y,Z,T).""" Do this for the
occurrences at the current docstrings (lines shown in the review: 25, 48, 67,
82) so each test clearly documents the expected input shape and axis ordering.
🪄 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: 59a6895d-b72f-4451-9129-bc49b00110e8
📒 Files selected for processing (5)
docs/API_MAP.mddocs/api/utilities/image_conversion.rstsrc/physiomotion4d/cli/convert_image_4d_to_3d.pysrc/physiomotion4d/convert_image_4d_to_3d.pytests/test_convert_image_4d_to_3d.py
✅ Files skipped from review due to trivial changes (2)
- docs/API_MAP.md
- docs/api/utilities/image_conversion.rst
| output_dir = test_directories["output"] / "convert_image_4d_to_3d" | ||
| output_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
Shared output directory makes these tests order-dependent.
All tests write into the same folder, so stale slice_*.mha files can satisfy assertions even if a conversion in the current test fails. Use per-test subdirectories (or pre-clean matching files) for isolation.
Proposed fix
- output_dir = test_directories["output"] / "convert_image_4d_to_3d"
+ output_dir = (
+ test_directories['output'] / 'convert_image_4d_to_3d' / 'test_convert_4d_to_3d'
+ )
@@
- output_dir = test_directories["output"] / "convert_image_4d_to_3d"
+ output_dir = (
+ test_directories['output'] / 'convert_image_4d_to_3d' / 'test_slice_files_created'
+ )
@@
- output_dir = test_directories["output"] / "convert_image_4d_to_3d"
+ output_dir = (
+ test_directories['output'] / 'convert_image_4d_to_3d' / 'test_save_3d_images'
+ )Also applies to: 49-50, 83-84
🤖 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/test_convert_image_4d_to_3d.py` around lines 26 - 27, The tests share a
single output_dir (variable output_dir built from test_directories["output"] /
"convert_image_4d_to_3d") which makes tests order-dependent; modify each test to
create an isolated per-test subdirectory (e.g., append a unique name derived
from the test function name, a UUID, or use pytest's tmp_path fixture) before
calling output_dir.mkdir(...) or alternatively remove any existing slice_*.mha
files in that directory before assertions; update the three places that
construct output_dir (the occurrences around output_dir =
test_directories["output"] / "convert_image_4d_to_3d" and its siblings) so each
test writes to its own fresh folder to avoid stale files satisfying assertions.
| source = parser.add_mutually_exclusive_group(required=True) | ||
| source.add_argument( | ||
| "--input-image", | ||
| help=( | ||
| "Path to a 3D or 4D image file (ITK-readable, e.g. NRRD/NIfTI/MHA) " | ||
| "or a directory containing a DICOM series (3D or 4D)." | ||
| ), | ||
| ) | ||
| parser.add_argument( | ||
| "--output-dir", | ||
| required=True, | ||
| help="Directory for output .mha files (created if absent).", | ||
| ) | ||
| parser.add_argument( | ||
| "--basename", | ||
| default="slice", | ||
| help="Filename stem for each output frame (default: slice).", | ||
| ) | ||
| parser.add_argument( | ||
| "--suffix", | ||
| default="mha", | ||
| help="Suffix for each output frame (default: mha).", | ||
| ) | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| if args.input_image and not os.path.exists(args.input_image): | ||
| print(f"Error: input image not found: {args.input_image}") | ||
| return 1 | ||
| try: | ||
| from physiomotion4d import ConvertImage4DTo3D | ||
|
|
||
| converter = ConvertImage4DTo3D() | ||
| if args.input_image: | ||
| print(f"Loading 4D image: {args.input_image}") | ||
| converter.load_image_4d(args.input_image) |
| iop = np.asarray(entries[0][1].ImageOrientationPatient, dtype=float) | ||
| slice_normal = np.cross(iop[:3], iop[3:6]) | ||
|
|
||
| def proj(ds: pydicom.Dataset) -> float: | ||
| ipp = np.asarray(ds.ImagePositionPatient, dtype=float) | ||
| return float(np.dot(ipp, slice_normal)) | ||
|
|
||
| self.img_3d = [] | ||
| for key in sorted_keys: | ||
| ordered = sorted(groups[key], key=lambda item: proj(item[1])) | ||
| filenames = [fname for fname, _ in ordered] | ||
| self.img_3d.append(itk.imread(filenames)) |
| Slices are grouped by a composite key built from the DICOM tags | ||
| listed in ``self.dicom_phase_keys`` (the default set covers | ||
| ``TemporalPositionIdentifier``, ``TriggerTime``, the cardiac trigger | ||
| delay / phase tags, ``FrameReferenceDateTime``, ``AcquisitionTime``, | ||
| and ``ScanOptions``). Any tag whose | ||
| value differs between slices will split them into separate phases; | ||
| missing tags fall back to the per-tag default. When none of the | ||
| configured tags differ across slices, all slices form a single 3D | ||
| volume. Non-DICOM files and files without the geometry tags are | ||
| skipped. |
| | `vtk_to_usd/` subpackage | 2,657 | Low-level VTK -> USD building blocks (9 files) | | ||
| | `cli/` subpackage | 1,788 | CLI entry-point scripts (8 commands) | | ||
| | `workflow_convert_heart_gated_ct_to_usd.py` | ~540 | Heart CT to USD workflow | | ||
| | `workflow_convert_image_to_usd.py` | ~540 | Heart CT to USD workflow | |
|
|
||
| - **Workflow Classes**: Complete end-to-end pipeline processors | ||
| - `WorkflowConvertHeartGatedCTToUSD`: Heart-gated CT to USD processing workflow | ||
| - `WorkflowConvertImageToUSD`: Heart-gated CT to USD processing workflow |
| @@ -358,8 +358,8 @@ def test_images( | |||
| slice_007 = data_dir / "slice_007.mha" | |||
| if not slice_000.exists() or not slice_007.exists(): | |||
| print("\nConverting 4D NRRD to 3D time series...") | |||
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test_cli_smoke.py (1)
11-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd smoke test for the new
convert_image_4d_to_3dCLI module.Layer 2 of this PR adds a new CLI entry point (
physiomotion4d.cli.convert_image_4d_to_3d), but it is not included in theCLI_MODULESlist. This smoke test is the primary validation that each CLI module can handle--helpwithout import errors or crashes.✅ Proposed fix to add the missing CLI module
CLI_MODULES = [ "physiomotion4d.cli.convert_ct_to_vtk", + "physiomotion4d.cli.convert_image_4d_to_3d", "physiomotion4d.cli.convert_image_to_usd", "physiomotion4d.cli.convert_vtk_to_usd",🤖 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/test_cli_smoke.py` around lines 11 - 19, The CLI_MODULES list in tests/test_cli_smoke.py is missing the new entry point; add "physiomotion4d.cli.convert_image_4d_to_3d" to the CLI_MODULES list so the smoke test attempts to import and run --help for the new CLI module (update the list where CLI_MODULES is defined to include the new module name).src/physiomotion4d/cli/convert_image_to_usd.py (1)
36-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCLI input help is narrower than actual supported formats.
Line 36 still advertises only NRRD inputs. That conflicts with the generalized image converter behavior and can mislead users.
Suggested fix
- parser.add_argument( - "input_files", nargs="+", help="Path to 4D NRRD file or list of 3D NRRD files" - ) + parser.add_argument( + "input_files", + nargs="+", + help=( + "Path to a single 4D image source (e.g., ITK-readable file or DICOM series) " + "or a list of 3D image files as a time series" + ), + )🤖 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 `@src/physiomotion4d/cli/convert_image_to_usd.py` around lines 36 - 37, The CLI help for the argparse argument "input_files" is too specific to NRRD; update the help text on the add_argument call that defines "input_files" in convert_image_to_usd.py to describe all supported input types (e.g., 4D NRRD, list of 3D NRRD, and other supported image formats like NIfTI/DICOM/image stacks or whatever the image converter accepts) so it accurately reflects the generalized converter behavior.
♻️ Duplicate comments (1)
src/physiomotion4d/convert_image_4d_to_3d.py (1)
135-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStill need ndim guards before indexing NRRD header arrays.
Line 137 reads
space_directions.shape[1]before provingspace_directionsis 2-D, and Line 155 assumesmeasurement_frame[x]returns 3-vectors.pynrrdcan surface headers likespace directions: none (...) (...) (...)as 1-D object arrays, so this still fails on valid.seq.nrrdinputs withIndexErroror an invalid direction matrix.🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` around lines 135 - 155, The code currently indexes into space_directions and measurement_frame without verifying they are 2-D and that their rows are vectors; update the guards to check ndim and per-row lengths before indexing: ensure np.asarray(header["space directions"]) has ndim == 2 and shape[0] >= 4 and shape[1] >= 3 (otherwise raise a clear ValueError referencing filename and 'space directions'), and ensure np.asarray(header["measurement frame"]) has ndim == 2 and shape[0] >= 3 and each row has length >= 3 (otherwise raise a clear ValueError referencing filename and 'measurement frame'); then compute spacing_3d and direction_3d only after these checks, using e.g. spacing_3d = np.array([abs(float(space_directions[i + 1][i])) for i in range(3)], dtype=float) and direction_3d = np.array([np.asarray(measurement_frame[i])[:3].astype(float) for i in range(3)], dtype=float) so indexing into space_directions, measurement_frame, spacing_3d, and direction_3d is safe and correctly handles "none" or 1-D header entries.
🧹 Nitpick comments (3)
docs/api/workflows.rst (1)
44-53: ⚡ Quick winRename this section to match the generalized workflow name.
The section is still titled “Heart-Gated CT to USD”, but it now documents
WorkflowConvertImageToUSD, which is broader than heart-gated CT only. Please rename the section header to avoid narrowing the documented scope.🤖 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 `@docs/api/workflows.rst` around lines 44 - 53, The section header currently titled "Heart-Gated CT to USD" should be renamed to a generalized title that matches the documented class WorkflowConvertImageToUSD; update the reST section header above the autoclass block to something like "Convert Image to USD" (or "Image to USD Workflow") so the heading reflects the broader scope and aligns with the WorkflowConvertImageToUSD class documentation.src/physiomotion4d/workflow_convert_image_to_usd.py (2)
51-56: ⚡ Quick win
__init__input description is outdated for the generalized workflow.Line 54 still says inputs are 3D/4D NRRD files, but this workflow now routes through
ConvertImage4DTo3Dand supports broader ITK/DICOM paths. Please update the docstring to keep the public contract factual.As per coding guidelines, "Update docstrings for every changed public method. Keep claims factual."
🤖 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 `@src/physiomotion4d/workflow_convert_image_to_usd.py` around lines 51 - 56, The __init__ docstring for the image-to-USD workflow is outdated: update the docstring on the __init__ method in workflow_convert_image_to_usd.py to accurately describe that input_filenames can be 3D or 4D image sources in various formats (NRRD, ITK-supported formats, DICOM series, etc.), that single-file or multi-file inputs will be routed through the ConvertImage4DTo3D step, and keep the public contract factual (remove the narrow "3D NRRD" wording and mention contrast_enhanced and other parameters as before).
2-5: ⚡ Quick winDocument explicit 4D axis order in module docs.
This module docstring references 4D image handling but does not state the expected shape/order. Please add the explicit contract
(X, Y, Z, T)to avoid ambiguity.As per coding guidelines, "State axis order and shape explicitly in every docstring and comment that touches arrays."
🤖 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 `@src/physiomotion4d/workflow_convert_image_to_usd.py` around lines 2 - 5, Update the module docstring in workflow_convert_image_to_usd.py to explicitly state the 4D axis order/shape as "(X, Y, Z, T)" (e.g., "4D images are expected to have shape (X, Y, Z, T)"). Also scan this module's docstrings and any comments that touch array shapes (e.g., functions or classes that process 4D CT volumes) and add the same explicit axis-order contract so every array-related docstring states the shape as (X, Y, Z, T).
🤖 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 `@src/physiomotion4d/__init__.py`:
- Around line 72-73: You removed root-level exported class names which breaks
existing imports; reintroduce deprecated aliases that map the old names to the
new classes (e.g., create aliases pointing the old exported names to
WorkflowConvertImageToUSD and WorkflowConvertVTKToUSD) and emit a
DeprecationWarning via the warnings module on import so users see a one-release
notice; apply the same alias+warning approach for the other renamed exports in
this module to maintain backward compatibility for one release cycle.
In `@src/physiomotion4d/convert_image_4d_to_3d.py`:
- Around line 151-162: The spacing and direction extraction is incorrect: stop
using measurement_frame and diagonal entries; instead compute spacing_3d as the
Euclidean norms of the first three space_directions vectors and compute
direction_3d as those vectors normalized (preserving their full 3-component
orientation) before applying any axis flips from header.get("space", "").
Concretely, replace the existing spacing_3d and direction_3d logic with:
spacing_3d = np.array([np.linalg.norm(space_directions[i][:3]) for i in
range(3)], dtype=float) and direction_3d = np.array([space_directions[i][:3] /
(np.linalg.norm(space_directions[i][:3]) or 1.0) for i in range(3)],
dtype=float), then apply the "right"/"anterior"/"inferior" sign flips to the
appropriate components of direction_3d (e.g., direction_3d[0][0] *= -1 etc.) as
before.
- Around line 191-192: The DICOM reader returns images in LPS but we must store
RAS in self.img_3d and expose RAS via get_3d_image(); after the itk.imread call
that populates the temporary 3D image (the variable assigned before storing to
self.img_3d in the conversion path inside ConvertImage4DTo3D class), convert
LPS→RAS by flipping X and Y axes and adjusting the image direction and origin:
apply a sign flip to the first two direction columns (multiply the first and
second column vectors of the image direction matrix by -1) and negate the X and
Y origin components (or equivalently transform origin with the same
sign-change), perform the corresponding voxel-axis flips so the pixel data
orientation matches the new direction, then assign that adjusted image to
self.img_3d so get_3d_image() returns RAS; ensure all original spacing and
metadata are preserved.
In `@src/physiomotion4d/workflow_convert_image_to_usd.py`:
- Around line 209-217: Before calling self.converter.load_image_4d(...) reset
the cached time-series state: clear or reinitialize self._time_series_images and
set self._num_time_points = 0 so subsequent appends don't duplicate stale
frames; then proceed with load_image_4d, save_3d_images, self._num_time_points =
self.converter.get_number_of_3d_images(), and the loop calling
self.converter.get_3d_image(i).
---
Outside diff comments:
In `@src/physiomotion4d/cli/convert_image_to_usd.py`:
- Around line 36-37: The CLI help for the argparse argument "input_files" is too
specific to NRRD; update the help text on the add_argument call that defines
"input_files" in convert_image_to_usd.py to describe all supported input types
(e.g., 4D NRRD, list of 3D NRRD, and other supported image formats like
NIfTI/DICOM/image stacks or whatever the image converter accepts) so it
accurately reflects the generalized converter behavior.
In `@tests/test_cli_smoke.py`:
- Around line 11-19: The CLI_MODULES list in tests/test_cli_smoke.py is missing
the new entry point; add "physiomotion4d.cli.convert_image_4d_to_3d" to the
CLI_MODULES list so the smoke test attempts to import and run --help for the new
CLI module (update the list where CLI_MODULES is defined to include the new
module name).
---
Duplicate comments:
In `@src/physiomotion4d/convert_image_4d_to_3d.py`:
- Around line 135-155: The code currently indexes into space_directions and
measurement_frame without verifying they are 2-D and that their rows are
vectors; update the guards to check ndim and per-row lengths before indexing:
ensure np.asarray(header["space directions"]) has ndim == 2 and shape[0] >= 4
and shape[1] >= 3 (otherwise raise a clear ValueError referencing filename and
'space directions'), and ensure np.asarray(header["measurement frame"]) has ndim
== 2 and shape[0] >= 3 and each row has length >= 3 (otherwise raise a clear
ValueError referencing filename and 'measurement frame'); then compute
spacing_3d and direction_3d only after these checks, using e.g. spacing_3d =
np.array([abs(float(space_directions[i + 1][i])) for i in range(3)],
dtype=float) and direction_3d =
np.array([np.asarray(measurement_frame[i])[:3].astype(float) for i in range(3)],
dtype=float) so indexing into space_directions, measurement_frame, spacing_3d,
and direction_3d is safe and correctly handles "none" or 1-D header entries.
---
Nitpick comments:
In `@docs/api/workflows.rst`:
- Around line 44-53: The section header currently titled "Heart-Gated CT to USD"
should be renamed to a generalized title that matches the documented class
WorkflowConvertImageToUSD; update the reST section header above the autoclass
block to something like "Convert Image to USD" (or "Image to USD Workflow") so
the heading reflects the broader scope and aligns with the
WorkflowConvertImageToUSD class documentation.
In `@src/physiomotion4d/workflow_convert_image_to_usd.py`:
- Around line 51-56: The __init__ docstring for the image-to-USD workflow is
outdated: update the docstring on the __init__ method in
workflow_convert_image_to_usd.py to accurately describe that input_filenames can
be 3D or 4D image sources in various formats (NRRD, ITK-supported formats, DICOM
series, etc.), that single-file or multi-file inputs will be routed through the
ConvertImage4DTo3D step, and keep the public contract factual (remove the narrow
"3D NRRD" wording and mention contrast_enhanced and other parameters as before).
- Around line 2-5: Update the module docstring in
workflow_convert_image_to_usd.py to explicitly state the 4D axis order/shape as
"(X, Y, Z, T)" (e.g., "4D images are expected to have shape (X, Y, Z, T)"). Also
scan this module's docstrings and any comments that touch array shapes (e.g.,
functions or classes that process 4D CT volumes) and add the same explicit
axis-order contract so every array-related docstring states the shape as (X, Y,
Z, T).
🪄 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: 703c8dd3-2338-4c07-9564-ca7a703295d0
📒 Files selected for processing (32)
README.mddocs/API_MAP.mddocs/PYPI_RELEASE_GUIDE.mddocs/api/cli/convert_heart_gated_ct_to_usd.rstdocs/api/cli/convert_image_to_usd.rstdocs/api/cli/index.rstdocs/api/index.rstdocs/api/workflows.rstdocs/architecture.rstdocs/cli_scripts/best_practices.rstdocs/cli_scripts/heart_gated_ct.rstdocs/cli_scripts/overview.rstdocs/developer/architecture.rstdocs/developer/workflows.rstdocs/examples.rstdocs/installation.rstdocs/quickstart.rstdocs/troubleshooting.rstdocs/tutorials.rstexperiments/Heart-Simpleware_Segmentation/README.mdexperiments/README.mdpyproject.tomlsrc/physiomotion4d/__init__.pysrc/physiomotion4d/cli/__init__.pysrc/physiomotion4d/cli/convert_image_4d_to_3d.pysrc/physiomotion4d/cli/convert_image_to_usd.pysrc/physiomotion4d/convert_image_4d_to_3d.pysrc/physiomotion4d/workflow_convert_image_to_usd.pystatistics.mdtests/test_cli_smoke.pytutorials/README.mdtutorials/tutorial_01_heart_gated_ct_to_usd.py
💤 Files with no reviewable changes (1)
- docs/api/cli/convert_heart_gated_ct_to_usd.rst
✅ Files skipped from review due to trivial changes (14)
- docs/api/index.rst
- docs/api/cli/index.rst
- docs/developer/workflows.rst
- tutorials/README.md
- docs/developer/architecture.rst
- docs/api/cli/convert_image_to_usd.rst
- docs/troubleshooting.rst
- docs/tutorials.rst
- docs/installation.rst
- statistics.md
- docs/cli_scripts/heart_gated_ct.rst
- docs/cli_scripts/overview.rst
- docs/API_MAP.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/architecture.rst
- src/physiomotion4d/cli/init.py
| from .workflow_convert_image_to_usd import WorkflowConvertImageToUSD | ||
| from .workflow_convert_vtk_to_usd import WorkflowConvertVTKToUSD |
There was a problem hiding this comment.
Root-level export rename is breaking without transition aliases.
Removing the old class names from package exports will break existing imports immediately. Please keep deprecated aliases for at least one release cycle and warn users.
Suggested compatibility bridge
from .workflow_convert_image_to_usd import WorkflowConvertImageToUSD
+WorkflowConvertHeartGatedCTToUSD = WorkflowConvertImageToUSD # deprecated alias
from .convert_image_4d_to_3d import ConvertImage4DTo3D
+ConvertNRRD4DTo3D = ConvertImage4DTo3D # deprecated alias
__all__ = [
@@
"WorkflowConvertImageToUSD",
+ "WorkflowConvertHeartGatedCTToUSD",
@@
"ConvertImage4DTo3D",
+ "ConvertNRRD4DTo3D",
]Also applies to: 83-84, 113-114
🤖 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 `@src/physiomotion4d/__init__.py` around lines 72 - 73, You removed root-level
exported class names which breaks existing imports; reintroduce deprecated
aliases that map the old names to the new classes (e.g., create aliases pointing
the old exported names to WorkflowConvertImageToUSD and WorkflowConvertVTKToUSD)
and emit a DeprecationWarning via the warnings module on import so users see a
one-release notice; apply the same alias+warning approach for the other renamed
exports in this module to maintain backward compatibility for one release cycle.
| spacing_3d = np.array( | ||
| [abs(space_directions[x + 1][x]) for x in range(3)], | ||
| dtype=float, | ||
| ) | ||
| direction_3d = np.array([measurement_frame[x] for x in range(3)], dtype=float) | ||
| space = header.get("space", "") | ||
| if "right" in space: | ||
| direction_3d[0][0] *= -1 | ||
| if "anterior" in space: | ||
| direction_3d[1][1] *= -1 | ||
| if "inferior" in space: | ||
| direction_3d[2][2] *= -1 |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | head -200Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 0
stdout:
1 """Convert a 3D or 4D image into a sequence of 3D images.
2
3 Reads a 3D or 4D medical image and, for the 4D case, splits it along the
4 temporal axis into individual 3D ITK volumes. Origin, spacing, and direction
5 are preserved in each per-frame volume. A pure 3D input becomes a one-element
6 time series.
7
8 Three reader paths are used:
9
10 * A *directory* path is treated as a DICOM series and read with ``pydicom``.
11 The slices are grouped by temporal position (``TemporalPositionIdentifier``
12 or ``TriggerTime``); each group yields one 3D ITK image. Directories that
13 contain a single phase produce a single 3D image.
14 * ``.nrrd`` files: 4D Slicer ``.seq.nrrd`` heart sequences (whose per-voxel
15 vector dimension exceeds ITK Python's wrapped Vector sizes) go through
16 ``pynrrd``. Plain 3D NRRDs fall back to ``itk.imread``.
17 * Every other format goes through ``itk.imread`` and may be either 3D or 4D
18 (e.g. NIfTI ``.nii.gz`` with ``dim[0] == 3`` or ``4``).
19 """
20
21 import logging
22 from collections import defaultdict
23 from pathlib import Path
24 from typing import Any, Union
25
26 import itk
27 import nrrd
28 import numpy as np
29 import pydicom
30
31 from physiomotion4d.physiomotion4d_base import PhysioMotion4DBase
32
33
34 class ConvertImage4DTo3D(PhysioMotion4DBase):
35 """Split a 3D/4D ITK image (X, Y, Z [, T]) into a list of 3D ITK images."""
36
37 def __init__(self, log_level: int | str = logging.INFO) -> None:
38 """Initialize the 4D-to-3D image converter.
39
40 Args:
41 log_level: Logging level (default: logging.INFO)
42 """
43 super().__init__(class_name=self.__class__.__name__, log_level=log_level)
44 self.img_3d: list[itk.Image] = []
45
46 # Public DICOM tags used to split a 4D series into 3D phases. Maps
47 # tag keyword → default value (the value type also implies how the
48 # tag is parsed: ``float`` for numeric tags, ``str`` for string tags).
49 # External users may add, remove, or replace entries to tune phase
50 # grouping for their vendor-specific exports.
51 self.dicom_phase_keys: dict[str, Union[float, str]] = {
52 "TemporalPositionIdentifier": 0.0,
53 "TriggerTime": 0.0,
54 "NominalCardiacTriggerDelayTime": 0.0,
55 "ActualCardiacTriggerDelayTime": 0.0,
56 "NominalPercentageOfCardiacPhase": 0.0,
57 "FrameReferenceDateTime": "",
58 # "AcquisitionTime": "",
59 "ScanOptions": "",
60 }
61
62 def load_image_4d(self, filename: Union[str, Path]) -> None:
63 """Load a 3D or 4D image and populate ``self.img_3d`` with 3D frames.
64
65 Dispatch rules:
66
67 * A *directory* path is read as a DICOM series via ``pydicom``.
68 Slices are grouped by temporal phase; each group becomes one 3D
69 ITK image. A 3D-only directory produces a single 3D image.
70 * ``.nrrd`` files use ``pynrrd`` for true 4D Slicer ``.seq.nrrd``
71 inputs and fall back to ``itk.imread`` for plain 3D NRRDs.
72 * All other formats go through ``itk.imread``; the array may be
73 3D or 4D and is treated uniformly as a (1 or T)-frame sequence.
74
75 Args:
76 filename: Path to a 3D/4D image file, or a DICOM series directory.
77 """
78 path = Path(filename)
79 if path.is_dir():
80 self._load_dicom_directory(path)
81 return
82
83 name = str(path)
84 if name.lower().endswith(".nrrd"):
85 data, header = nrrd.read(name)
86 arr_data = np.asarray(data)
87 if arr_data.ndim == 4:
88 self._load_nrrd_4d(name, arr_data, header)
89 return
90 if arr_data.ndim != 3:
91 raise ValueError(
92 f"Expected 3D or 4D NRRD, got array shape {arr_data.shape}: {name}"
93 )
94 # 3D NRRD: defer to the standard ITK reader for correctness.
95
96 self._load_itk_file(name)
97
98 def _load_itk_file(self, filename: str) -> None:
99 """Read a 3D or 4D image with ``itk.imread`` and slice along T."""
100 img = itk.imread(filename)
101 arr = itk.array_view_from_image(img)
102 if arr.ndim not in (3, 4):
103 raise ValueError(
104 f"Expected a 3D or 4D image, got array shape {arr.shape}: {filename}"
105 )
106 origin_3d = np.asarray(img.GetOrigin())[:3]
107 spacing_3d = np.asarray(img.GetSpacing())[:3]
108 direction_3d = itk.array_from_matrix(img.GetDirection())[:3, :3]
109
110 if arr.ndim == 3:
111 arr_4d = arr[np.newaxis, ...]
112 else:
113 arr_4d = arr
114
115 self._build_frames(arr_4d, origin_3d, spacing_3d, direction_3d)
116
117 def _load_nrrd_4d(
118 self,
119 filename: str,
120 data: np.ndarray,
121 header: dict[str, Any],
122 ) -> None:
123 """Build per-frame 3D ITK images from a Slicer 4D ``.seq.nrrd``."""
124 # pynrrd returns the data in (T, X, Y, Z) order for a 4D NRRD.
125 # ITK numpy views use (T, Z, Y, X) — transpose the spatial axes.
126 arr_4d = np.ascontiguousarray(data.transpose(0, 3, 2, 1))
127
128 required_keys = ("space origin", "space directions", "measurement frame")
129 missing = [k for k in required_keys if k not in header]
130 if missing:
131 raise ValueError(
132 f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
133 f"missing NRRD header field(s) {missing}"
134 )
135 space_directions = np.asarray(header["space directions"])
136 measurement_frame = np.asarray(header["measurement frame"])
137 if space_directions.shape[0] < 4 or space_directions.shape[1] < 3:
138 raise ValueError(
139 f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
140 f"'space directions' has shape {space_directions.shape}, "
141 "expected at least (4, 3)"
142 )
143 if measurement_frame.shape[0] < 3:
144 raise ValueError(
145 f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
146 f"'measurement frame' has shape {measurement_frame.shape}, "
147 "expected at least 3 rows"
148 )
149
150 origin_3d = np.asarray(header["space origin"], dtype=float)
151 spacing_3d = np.array(
152 [abs(space_directions[x + 1][x]) for x in range(3)],
153 dtype=float,
154 )
155 direction_3d = np.array([measurement_frame[x] for x in range(3)], dtype=float)
156 space = header.get("space", "")
157 if "right" in space:
158 direction_3d[0][0] *= -1
159 if "anterior" in space:
160 direction_3d[1][1] *= -1
161 if "inferior" in space:
162 direction_3d[2][2] *= -1
163
164 self._build_frames(arr_4d, origin_3d, spacing_3d, direction_3d)
165
166 def _build_frames(
167 self,
168 arr_4d: np.ndarray,
169 origin_3d: np.ndarray,
170 spacing_3d: np.ndarray,
171 direction_3d: np.ndarray,
172 ) -> None:
173 """Materialize ``self.img_3d`` from a (T, Z, Y, X) array + geometry."""
174 direction_matrix = itk.matrix_from_array(np.ascontiguousarray(direction_3d))
175 self.img_3d = []
176 for t in range(arr_4d.shape[0]):
177 # Copy so each 3D image owns its buffer independently.
178 arr_3d = np.ascontiguousarray(arr_4d[t])
179 img3d = itk.image_from_array(arr_3d)
180 img3d.SetOrigin(origin_3d.tolist())
181 img3d.SetSpacing(spacing_3d.tolist())
182 img3d.SetDirection(direction_matrix)
183 self.img_3d.append(img3d)
184
185 def _load_dicom_directory(self, dirpath: Path) -> None:
186 """Read a DICOM directory and build one 3D image per temporal phase.
187
188 Files in ``dirpath`` are inspected with ``pydicom`` to identify valid
189 DICOM image slices, group them by temporal phase, and sort them along
190 the slice normal. The resulting ordered filename list for each phase
191 is handed to ``itk.imread``, which constructs the 3D image with proper
192 origin, spacing, and direction in LPS world space via its DICOM IO.
193
194 Slices are grouped by a composite key built from the DICOM tags
195 listed in ``self.dicom_phase_keys`` (the default set covers
196 ``TemporalPositionIdentifier``, ``TriggerTime``, the cardiac trigger
197 delay / phase tags, ``FrameReferenceDateTime``, ``AcquisitionTime``,
198 and ``ScanOptions``). Any tag whose
199 value differs between slices will split them into separate phases;
200 missing tags fall back to the per-tag default. When none of the
Script executed:
# Look for test files related to convert_image_4d_to_3d
find . -name "*test*" -type f -exec grep -l "convert_image_4d_to_3d\|ConvertImage4DTo3D" {} \;Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 0
stdout:
./docs/testing.rst
./tests/conftest.py
./tests/test_convert_image_4d_to_3d.py
./tests/test_register_images_ants.py
./tests/test_register_images_icon.py
./tests/test_segment_chest_total_segmentator.py
Script executed:
# Search for NRRD-related comments or documentation
rg -i "space.directions|measurement.frame" -A 3 -B 3Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 0
stdout:
src/physiomotion4d/convert_image_4d_to_3d.py- # ITK numpy views use (T, Z, Y, X) — transpose the spatial axes.
src/physiomotion4d/convert_image_4d_to_3d.py- arr_4d = np.ascontiguousarray(data.transpose(0, 3, 2, 1))
src/physiomotion4d/convert_image_4d_to_3d.py-
src/physiomotion4d/convert_image_4d_to_3d.py: required_keys = ("space origin", "space directions", "measurement frame")
src/physiomotion4d/convert_image_4d_to_3d.py- missing = [k for k in required_keys if k not in header]
src/physiomotion4d/convert_image_4d_to_3d.py- if missing:
src/physiomotion4d/convert_image_4d_to_3d.py- raise ValueError(
src/physiomotion4d/convert_image_4d_to_3d.py- f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
src/physiomotion4d/convert_image_4d_to_3d.py- f"missing NRRD header field(s) {missing}"
src/physiomotion4d/convert_image_4d_to_3d.py- )
src/physiomotion4d/convert_image_4d_to_3d.py: space_directions = np.asarray(header["space directions"])
src/physiomotion4d/convert_image_4d_to_3d.py: measurement_frame = np.asarray(header["measurement frame"])
src/physiomotion4d/convert_image_4d_to_3d.py: if space_directions.shape[0] < 4 or space_directions.shape[1] < 3:
src/physiomotion4d/convert_image_4d_to_3d.py- raise ValueError(
src/physiomotion4d/convert_image_4d_to_3d.py- f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
src/physiomotion4d/convert_image_4d_to_3d.py: f"'space directions' has shape {space_directions.shape}, "
src/physiomotion4d/convert_image_4d_to_3d.py- "expected at least (4, 3)"
src/physiomotion4d/convert_image_4d_to_3d.py- )
src/physiomotion4d/convert_image_4d_to_3d.py: if measurement_frame.shape[0] < 3:
src/physiomotion4d/convert_image_4d_to_3d.py- raise ValueError(
src/physiomotion4d/convert_image_4d_to_3d.py- f"{filename!r} is not a valid Slicer 4D .seq.nrrd: "
src/physiomotion4d/convert_image_4d_to_3d.py: f"'measurement frame' has shape {measurement_frame.shape}, "
src/physiomotion4d/convert_image_4d_to_3d.py- "expected at least 3 rows"
src/physiomotion4d/convert_image_4d_to_3d.py- )
src/physiomotion4d/convert_image_4d_to_3d.py-
src/physiomotion4d/convert_image_4d_to_3d.py- origin_3d = np.asarray(header["space origin"], dtype=float)
src/physiomotion4d/convert_image_4d_to_3d.py- spacing_3d = np.array(
src/physiomotion4d/convert_image_4d_to_3d.py: [abs(space_directions[x + 1][x]) for x in range(3)],
src/physiomotion4d/convert_image_4d_to_3d.py- dtype=float,
src/physiomotion4d/convert_image_4d_to_3d.py- )
src/physiomotion4d/convert_image_4d_to_3d.py: direction_3d = np.array([measurement_frame[x] for x in range(3)], dtype=float)
src/physiomotion4d/convert_image_4d_to_3d.py- space = header.get("space", "")
src/physiomotion4d/convert_image_4d_to_3d.py- if "right" in space:
src/physiomotion4d/convert_image_4d_to_3d.py- direction_3d[0][0] *= -1
Script executed:
# Check if there are any example NRRD files or documentation
find . -name "*.md" -o -name "*.rst" | xargs grep -l "nrrd\|NRRD" 2>/dev/null | head -10Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 0
stdout:
./README.md
./data/README.md
./docs/API_MAP.md
./docs/api/segmentation/base.rst
./docs/api/segmentation/index.rst
./docs/api/segmentation/simpleware.rst
./docs/api/segmentation/totalsegmentator.rst
./docs/api/utilities/contour_tools.rst
./docs/api/utilities/image_conversion.rst
./docs/api/utilities/test_tools.rst
Script executed:
cat -n tests/test_convert_image_4d_to_3d.pyRepository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 0
stdout:
1 #!/usr/bin/env python
2 """
3 Test for converting a 4D image to a 3D time series using ITK readers.
4
5 This test depends on test_download_heart_data and replicates the functionality
6 from cell 3 of the notebook Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.ipynb.
7 """
8
9 from pathlib import Path
10
11 import pytest
12
13 from physiomotion4d.convert_image_4d_to_3d import ConvertImage4DTo3D
14
15
16 `@pytest.mark.requires_data`
17 class TestConvertImage4DTo3D:
18 """Test suite for converting a 4D image to a 3D time series."""
19
20 def test_convert_4d_to_3d(
21 self,
22 download_test_data: Path,
23 test_directories: dict[str, Path],
24 ) -> None:
25 """Test conversion of 4D image to 3D time series."""
26 output_dir = test_directories["output"] / "convert_image_4d_to_3d"
27 output_dir.mkdir(parents=True, exist_ok=True)
28
29 input_4d_file = download_test_data
30
31 print("\nConverting 4D image to 3D time series...")
32 conv = ConvertImage4DTo3D()
33 conv.load_image_4d(str(input_4d_file))
34 conv.save_3d_images(output_dir, "slice")
35
36 slice_007 = output_dir / "slice_007.mha"
37 assert slice_007.exists(), f"Expected slice file not created: {slice_007}"
38
39 slice_files = list(output_dir.glob("slice_*.mha"))
40 print(f"Created {len(slice_files)} slice files")
41 assert len(slice_files) > 0, "No slice files were created"
42
43 def test_slice_files_created(
44 self,
45 download_test_data: Path,
46 test_directories: dict[str, Path],
47 ) -> None:
48 """Test that all expected slice files are present after conversion."""
49 output_dir = test_directories["output"] / "convert_image_4d_to_3d"
50 output_dir.mkdir(parents=True, exist_ok=True)
51
52 conv = ConvertImage4DTo3D()
53 conv.load_image_4d(str(download_test_data))
54 conv.save_3d_images(output_dir, "slice")
55
56 slice_files = list(output_dir.glob("slice_*.mha"))
57 assert len(slice_files) > 10, (
58 f"Expected more than 10 slice files, found {len(slice_files)}"
59 )
60
61 slice_007 = output_dir / "slice_007.mha"
62 assert slice_007.exists(), "Expected slice_007.mha not found"
63
64 print(f"\nFound {len(slice_files)} slice files")
65
66 def test_load_image_4d(self, download_test_data: Path) -> None:
67 """Test loading a 4D image."""
68 input_4d_file = download_test_data
69
70 conv = ConvertImage4DTo3D()
71 conv.load_image_4d(str(input_4d_file))
72
73 assert conv.get_number_of_3d_images() > 0, "No time points found in 4D image"
74
75 print(f"\nLoaded 4D image with {conv.get_number_of_3d_images()} time points")
76
77 def test_save_3d_images(
78 self,
79 download_test_data: Path,
80 test_directories: dict[str, Path],
81 ) -> None:
82 """Test saving 3D images from a 4D source."""
83 output_dir = test_directories["output"] / "convert_image_4d_to_3d"
84 output_dir.mkdir(parents=True, exist_ok=True)
85
86 input_4d_file = download_test_data
87
88 conv = ConvertImage4DTo3D()
89 conv.load_image_4d(str(input_4d_file))
90
91 num_time_points = conv.get_number_of_3d_images()
92
93 conv.save_3d_images(output_dir, "test_slice")
94
95 test_slice_files = list(output_dir.glob("test_slice_*.mha"))
96 assert len(test_slice_files) > 0, "No test slice files were created"
97 assert len(test_slice_files) == num_time_points, (
98 f"Expected {num_time_points} files, found {len(test_slice_files)}"
99 )
100
101 print(f"\nSaved {len(test_slice_files)} 3D images")
102
103 for test_file in test_slice_files:
104 test_file.unlink()
105
106
107 if __name__ == "__main__":
108 pytest.main([__file__, "-v", "-s"])
Script executed:
# Look for any comments or documentation about NRRD geometry
rg "oblique|space.directions|measurement" -i docs/Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 1
Script executed:
# Search for any references to NRRD geometry in Python files
rg "norm.*space|vector.*direction" -i src/Repository: Project-MONAI/physiomotion4d
Repository: Project-MONAI/physiomotion4d
Exit code: 1
NRRD space directions measurement frame geometry oblique
In the NRRD (Nearly Raw Raster Data) file format, the concepts of space directions and the measurement frame are distinct mechanisms used to define the geometry and orientation of the data [1][2]. Space Directions The space directions field is a per-axis specification that defines the mapping from image space (the array indices) to world space coordinates [1][2]. It consists of a set of vectors—one for each axis of the array—that describe the direction and spacing of that axis in world space [2][3]. When these vectors are not aligned with the primary axes of the world coordinate system, the data is considered to have an oblique orientation [4]. Measurement Frame The measurement frame is a basic (per-array) field specification that defines a coordinate system for interpreting vector or matrix-valued data [1][2]. It identifies a spaceDim-by-spaceDim matrix that transforms coordinates from the measurement frame to world space [2]. This is essential when the coefficients of a vector or matrix (such as diffusion tensors or gradient vectors) are measured with respect to a specific local frame rather than the global world space [1][2]. Relationship to Oblique Geometry While space directions handle the spatial positioning and orientation of the image grid itself (including oblique rotations relative to world space), the measurement frame handles the orientation of the data values stored at each voxel [1][2]. If a dataset is oblique, the space directions field accounts for this rotation so that the data can be correctly mapped to world space [2]. The measurement frame ensures that any directional data (like vectors) associated with those voxels is also correctly interpreted relative to that world space, regardless of the original measurement orientation [1][2].
Citations:
- 1: https://paulbourke.net/dataformats/nrrd/
- 2: https://teem.sourceforge.net/nrrd/format.html
- 3: https://pynrrd.readthedocs.io/en/stable/reference/reading.html
- 4: https://www.mathworks.com/help/medical-imaging/ref/medicalvolume.html
pynrrd space_directions spacing direction proper handling oblique
In the NRRD file format, space directions and spacing are distinct but related concepts used to define the spatial orientation and resolution of data [1][2]. Proper handling of oblique (non-orthogonal) data requires using the space directions field rather than simple scalar spacing [1][2]. Key concepts for handling these fields in pynrrd: 1. Space Directions vs. Spacing: The space directions field is a per-axis definition that specifies both the direction and the magnitude (spacing) of each axis in world space [3][1][2]. Each entry in space directions is a vector representing the difference in position associated with incrementing that axis by one [1][2]. Unlike DICOM direction cosines, these vectors are not unit-length; their magnitude corresponds to the physical spacing between samples [1][2]. In contrast, the spacings field (if used) typically only provides scalar values for orthogonal axes and cannot represent oblique orientations [4][1]. 2. Handling Oblique Data: To correctly represent oblique data, you must populate the space directions field with the appropriate transformation vectors [1][2]. If your data is oblique, the space directions matrix will contain non-zero off-diagonal elements [1][2]. When writing NRRD files with pynrrd, ensure these are provided as a list of vectors or a matrix that correctly maps your array axes to the desired world-space orientation [3]. 3. pynrrd Implementation: - Reading: pynrrd allows you to control how space directions are returned via the nrrd.SPACE_DIRECTIONS_TYPE configuration [3]. You can set this to 'double matrix' (default) or 'double vector list' to suit your processing pipeline [3]. - Writing: When writing, you can pass space directions directly in the header dictionary [5]. Ensure that the vectors provided are consistent with your data's orientation and that you are not inadvertently relying on default orthogonal assumptions [5][1]. - Index Order: Be mindful of index ordering (C-order vs. Fortran-order). The index order used during reading must be consistent with writing to avoid unintended transpositions of your data and orientation information [6]. If you encounter visualization issues in tools like 3D Slicer, verify that your space origin and space directions are correctly defined in the header, as these are critical for the software to correctly place the volume in world space [5][7].
Citations:
- 1: https://paulbourke.net/dataformats/nrrd/
- 2: https://teem.sourceforge.net/nrrd/format.html
- 3: https://pynrrd.readthedocs.io/en/stable/reference/reading.html
- 4: NRRD I/O: dealing with spacedirections JuliaImages/Images.jl#86
- 5: Write NRRD file with header 'space origin' has format bug mhe/pynrrd#82
- 6: https://pynrrd.readthedocs.io/en/stable/background/how-to-use.html
- 7: https://discourse.slicer.org/t/incorrect-volume-information-when-loading-nrrd/33907
ITK image direction matrix vs spacing how to derive from space_directions vectors
In the Insight Toolkit (ITK), an image's spatial geometry is defined by its origin, spacing, and direction matrix [1][2]. These components work together to map image index coordinates (i, j, k) to physical space coordinates (x, y, z) [3][4]. The relationship is defined by the following linear transformation [3]: P = O + (D * S) * I Where: - P is the physical coordinate vector. - O is the image origin (the physical location of the voxel at index). - D is the direction cosine matrix (an N x N orthonormal matrix). - S is the diagonal spacing matrix (a diagonal matrix where the diagonal elements are the spacing values for each dimension). - I is the image index vector. Key Concepts: 1. Spacing: Represents the physical distance between the centers of adjacent voxels along each dimension [5][2]. It is stored as a vector of positive values [2]. 2. Direction Matrix: An orthonormal matrix where each column represents the unit vector in physical space corresponding to the direction of an image axis [1][2]. Because it is orthonormal, the columns are unit vectors (length of 1) [3][4]. 3. Space Directions: In some file formats (like MetaIO/mhd), you may encounter "space directions" vectors. These are the columns of the direction matrix scaled by the spacing [4]. If you have a set of space direction vectors (let's call them V_0, V_1,... V_n), you can derive the spacing and direction matrix as follows: - The spacing for dimension i is the Euclidean norm (length) of the corresponding space direction vector V_i: Spacing[i] = ||V_i|| [4]. - The direction matrix D is formed by normalizing these vectors: D_i = V_i / ||V_i|| [4]. In summary, the direction matrix contains the orientation (unit vectors), while the spacing scales these vectors to represent the physical distance between voxels [3][4]. If you are working with raw "space directions" vectors, the length of each vector provides the spacing, and the normalized vector provides the corresponding column of the direction matrix [4].
Citations:
- 1: https://simpleitk.readthedocs.io/en/v1.2.3/Documentation/docs/source/fundamentalConcepts.html
- 2: https://insightsoftwareconsortium.github.io/ITKDoxygen/classitk_1_1ImageBase.html
- 3: https://docs.itk.org/projects/doxygen/en/latest/Examples_2DataRepresentation_2Image_2Image4_8cxx-example.html
- 4: http://itksnap.org/pmwiki/pmwiki.php?n=Documentation.DirectionMatrices
- 5: https://itk.org/ITKSoftwareGuide/html/Book1/ITKSoftwareGuide-Book1ch4.html
Fix NRRD geometry extraction to properly handle oblique data.
The current code extracts spacing from only the diagonal entries of space directions and incorrectly uses measurement frame for the image direction. This breaks any oblique (non-axis-aligned) geometry and conflates two distinct NRRD concepts:
- Space directions: Per-axis vectors defining the grid's mapping to world space (including oblique rotations). The magnitude of each vector is the physical spacing; the normalized vector is the spatial orientation.
- Measurement frame: Defines the coordinate system for interpreting vector/matrix data values at each voxel—unrelated to grid positioning.
Correct approach:
- Spacing should be derived from the Euclidean norms of the first three
space directionsvectors. - Direction matrix should be formed from the normalized components of those vectors (before any coordinate-system flipping).
🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` around lines 151 - 162, The
spacing and direction extraction is incorrect: stop using measurement_frame and
diagonal entries; instead compute spacing_3d as the Euclidean norms of the first
three space_directions vectors and compute direction_3d as those vectors
normalized (preserving their full 3-component orientation) before applying any
axis flips from header.get("space", ""). Concretely, replace the existing
spacing_3d and direction_3d logic with: spacing_3d =
np.array([np.linalg.norm(space_directions[i][:3]) for i in range(3)],
dtype=float) and direction_3d = np.array([space_directions[i][:3] /
(np.linalg.norm(space_directions[i][:3]) or 1.0) for i in range(3)],
dtype=float), then apply the "right"/"anterior"/"inferior" sign flips to the
appropriate components of direction_3d (e.g., direction_3d[0][0] *= -1 etc.) as
before.
| is handed to ``itk.imread``, which constructs the 3D image with proper | ||
| origin, spacing, and direction in LPS world space via its DICOM IO. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/physiomotion4d/convert_image_4d_to_3d.pyRepository: Project-MONAI/physiomotion4d
Length of output: 121
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '180,200p'Repository: Project-MONAI/physiomotion4d
Length of output: 1420
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '240,260p'Repository: Project-MONAI/physiomotion4d
Length of output: 1035
🏁 Script executed:
head -50 src/physiomotion4d/convert_image_4d_to_3d.pyRepository: Project-MONAI/physiomotion4d
Length of output: 2089
🏁 Script executed:
rg -i "lps|ras|coordinate" src/physiomotion4d/ -A 3 -B 1Repository: Project-MONAI/physiomotion4d
Length of output: 50386
🏁 Script executed:
rg -i "itk\.Image|itk\.imread" src/physiomotion4d/ --type py -B 2 -A 2 | head -100Repository: Project-MONAI/physiomotion4d
Length of output: 9339
🏁 Script executed:
rg -i "lps.*ras|ras.*lps" src/physiomotion4d/ --type py -B 2 -A 2Repository: Project-MONAI/physiomotion4d
Length of output: 1107
🏁 Script executed:
rg -i "flipaxes|flip|convert.*coordin" src/physiomotion4d/ --type pyRepository: Project-MONAI/physiomotion4d
Length of output: 4661
🏁 Script executed:
grep -r "def.*lps\|def.*ras\|ITK_COORDINATE\|DICOM.*coordinate" src/physiomotion4d/ --include="*.py"Repository: Project-MONAI/physiomotion4d
Length of output: 568
🏁 Script executed:
rg "ConvertImage4DTo3D\|convert_image_4d_to_3d" src/physiomotion4d/ --type py -B 2 -A 2Repository: Project-MONAI/physiomotion4d
Length of output: 54
🏁 Script executed:
cat -n src/physiomotion4d/image_tools.py | sed -n '1,50p'Repository: Project-MONAI/physiomotion4d
Length of output: 2011
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '100,170p'Repository: Project-MONAI/physiomotion4d
Length of output: 3381
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '250,260p'Repository: Project-MONAI/physiomotion4d
Length of output: 543
🏁 Script executed:
rg "img_3d\|get_3d_image" src/physiomotion4d/ --type py -B 1 -A 3Repository: Project-MONAI/physiomotion4d
Length of output: 54
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '185,260p'Repository: Project-MONAI/physiomotion4d
Length of output: 4049
🏁 Script executed:
rg "def load|def read" src/physiomotion4d/convert_image_4d_to_3d.py --type py -A 5Repository: Project-MONAI/physiomotion4d
Length of output: 313
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '1,100p'Repository: Project-MONAI/physiomotion4d
Length of output: 4822
🏁 Script executed:
rg "itk.imread" src/physiomotion4d/ --type py -B 3 -A 3 | head -80Repository: Project-MONAI/physiomotion4d
Length of output: 6856
🏁 Script executed:
cat -n src/physiomotion4d/convert_image_4d_to_3d.py | sed -n '166,184p'Repository: Project-MONAI/physiomotion4d
Length of output: 968
🏁 Script executed:
rg "class ConvertImage4DTo3D" src/physiomotion4d/ -A 30 --type pyRepository: Project-MONAI/physiomotion4d
Length of output: 2848
🏁 Script executed:
rg "LPS\|lps" src/physiomotion4d/ --type pyRepository: Project-MONAI/physiomotion4d
Length of output: 54
🏁 Script executed:
rg "convert.*dicom|dicom.*convert" src/physiomotion4d/ -i --type pyRepository: Project-MONAI/physiomotion4d
Length of output: 54
🏁 Script executed:
rg "def.*coordinate|coordinate.*system" src/physiomotion4d/ -i --type pyRepository: Project-MONAI/physiomotion4d
Length of output: 1633
🏁 Script executed:
web_search
ITK DICOM coordinate system LPS RASRepository: Project-MONAI/physiomotion4d
Length of output: 165
🌐 Web query:
ITK DICOM reader coordinate system LPS RAS
💡 Result:
In ITK’s DICOM reader, the physical (patient) coordinate convention is LPS (Left, Posterior, Superior). When you read DICOM, ITK uses the DICOM-provided direction cosines and origin expressed in LPS, and stores them in the image’s Direction and Origin so that the image lives in LPS space. [1][2][3] How LPS relates to RAS: - RAS (Right, Anterior, Superior) is the same anatomical axes as LPS but with the first two axes flipped (L/R and P/A sign inversion). [1][4] - Therefore, converting between ITK/LPS and a RAS-based convention is effectively a sign flip on X and Y (with Z unchanged), and the corresponding direction/origin transforms must be updated accordingly. This “flip” is what tools like 3D Slicer do when mapping between their internal RAS convention and DICOM/ITK LPS. [1][5] Practical takeaway: - If you are using ITK directly: treat the output image’s physical coordinates as LPS. - If you need RAS coordinates (e.g., for compatibility with Slicer or other RAS-based tooling): flip the first two axes (X and Y) when converting between LPS and RAS, and apply the same convention to both orientation (direction cosines) and origin/transform. [1][5]
Citations:
- 1: https://www.slicer.org/wiki/Coordinate_systems
- 2: https://discourse.itk.org/t/how-to-understand-the-coordinate-system-of-dicom/5180
- 3: https://insight-developers.itk.narkive.com/rMQfxKLJ/gdcm-itk-origin-and-coordinate-system
- 4: https://slicer.readthedocs.io/en/latest/user_guide/coordinate_systems.html
- 5: https://www.slicer.org/wiki/Slicer:VTK_ITK_Coordinates
Convert DICOM frames from LPS to RAS before storing in self.img_3d.
ITK's DICOM reader produces images in LPS (Left, Posterior, Superior) world space. Per the coding guidelines, images must be stored in RAS (Right, Anterior, Superior) space. The DICOM loading path at lines 251–255 directly appends itk.imread output without conversion. The docstring at lines 191–192 correctly documents the LPS output but the class then exposes these unconverted images via get_3d_image(), violating the RAS requirement.
Conversion requires flipping the X and Y axes and updating the direction matrix and origin accordingly.
🤖 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 `@src/physiomotion4d/convert_image_4d_to_3d.py` around lines 191 - 192, The
DICOM reader returns images in LPS but we must store RAS in self.img_3d and
expose RAS via get_3d_image(); after the itk.imread call that populates the
temporary 3D image (the variable assigned before storing to self.img_3d in the
conversion path inside ConvertImage4DTo3D class), convert LPS→RAS by flipping X
and Y axes and adjusting the image direction and origin: apply a sign flip to
the first two direction columns (multiply the first and second column vectors of
the image direction matrix by -1) and negate the X and Y origin components (or
equivalently transform origin with the same sign-change), perform the
corresponding voxel-axis flips so the pixel data orientation matches the new
direction, then assign that adjusted image to self.img_3d so get_3d_image()
returns RAS; ensure all original spacing and metadata are preserved.
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 `@src/physiomotion4d/vtk_to_usd/usd_utils.py`:
- Around line 70-71: The current validation does shape[1] checks on points and
normals which will raise IndexError for 1D arrays; update the checks to first
verify ndim==2 (e.g., use points.ndim != 2 or points.shape[1] != 3) and
similarly for normals (normals.ndim != 2 or normals.shape[1] != 3) so that
invalid dimensionality produces the intended ValueError; modify the existing
conditionals that reference points.shape[1] and normals.shape[1] to include the
ndim guard and keep the same ValueError messages.
🪄 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: bb24bbea-37db-4e01-8372-70a7897f3e12
📒 Files selected for processing (22)
.agents/agents/docs.md.agents/agents/implementation.mdCLAUDE.mdREADME.mddocs/API_MAP.mddocs/api/workflows.rstdocs/architecture.rstdocs/developer/architecture.rstdocs/developer/workflows.rstexperiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.pysrc/physiomotion4d/cli/convert_image_4d_to_3d.pysrc/physiomotion4d/cli/convert_image_to_usd.pysrc/physiomotion4d/convert_image_4d_to_3d.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/vtk_to_usd/CLAUDE.mdsrc/physiomotion4d/vtk_to_usd/__init__.pysrc/physiomotion4d/vtk_to_usd/usd_mesh_converter.pysrc/physiomotion4d/vtk_to_usd/usd_utils.pysrc/physiomotion4d/workflow_convert_image_to_usd.pystatistics.mdtests/conftest.pytests/test_cli_smoke.py
✅ Files skipped from review due to trivial changes (8)
- .agents/agents/implementation.md
- src/physiomotion4d/vtk_to_usd/CLAUDE.md
- .agents/agents/docs.md
- docs/api/workflows.rst
- CLAUDE.md
- experiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.py
- src/physiomotion4d/test_tools.py
- statistics.md
🚧 Files skipped from review as they are similar to previous changes (9)
- docs/developer/architecture.rst
- docs/architecture.rst
- src/physiomotion4d/cli/convert_image_4d_to_3d.py
- docs/developer/workflows.rst
- README.md
- src/physiomotion4d/cli/convert_image_to_usd.py
- tests/conftest.py
- src/physiomotion4d/workflow_convert_image_to_usd.py
- src/physiomotion4d/convert_image_4d_to_3d.py
| if points.shape[1] != 3: | ||
| raise ValueError(f"Points must have shape (N, 3), got {points.shape}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm current guards in file
rg -n 'def lps_points_to_usd|def lps_normals_to_usd|shape\[1\] != 3' src/physiomotion4d/vtk_to_usd/usd_utils.py
# Confirm why direct shape[1] access is unsafe for 1D arrays
py - <<'PY'
import numpy as np
a = np.array([1.0, 2.0, 3.0])
try:
_ = a.shape[1]
except Exception as e:
print(type(e).__name__)
PYRepository: Project-MONAI/physiomotion4d
Length of output: 320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read file context around the two function definitions
echo "=== File context around lps_points_to_usd ==="
sed -n '58,80p' src/physiomotion4d/vtk_to_usd/usd_utils.py
echo ""
echo "=== File context around lps_normals_to_usd ==="
sed -n '82,105p' src/physiomotion4d/vtk_to_usd/usd_utils.py
# Verify 1D array behavior using python
echo ""
echo "=== Verify 1D array shape indexing ==="
python3 <<'PY'
import numpy as np
a1d = np.array([1.0, 2.0, 3.0])
print(f"1D array shape: {a1d.shape}")
try:
_ = a1d.shape[1]
except IndexError as e:
print(f"Accessing shape[1] on 1D array raises: {type(e).__name__}")
except Exception as e:
print(f"Unexpected error: {type(e).__name__}: {e}")
PYRepository: Project-MONAI/physiomotion4d
Length of output: 1924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for calls to these functions
echo "=== Calls to lps_points_to_usd ==="
rg -n 'lps_points_to_usd' --type py -B2 -A2 src/
echo ""
echo "=== Calls to lps_normals_to_usd ==="
rg -n 'lps_normals_to_usd' --type py -B2 -A2 src/Repository: Project-MONAI/physiomotion4d
Length of output: 4916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find MeshData class definition
echo "=== Searching for MeshData class ==="
rg -n 'class MeshData' --type py src/
# Get context around MeshData definition
echo ""
echo "=== MeshData class definition ==="
rg -n -A20 'class MeshData' --type py src/Repository: Project-MONAI/physiomotion4d
Length of output: 2148
Guard dimension before shape[1] access to raise ValueError instead of IndexError.
Lines 70 and 94 index shape[1] directly without checking ndim, causing 1D inputs to raise IndexError instead of the intended validation ValueError. Add points.ndim != 2 or and normals.ndim != 2 or checks before the shape[1] comparisons.
Also applies to: 94–95
🤖 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 `@src/physiomotion4d/vtk_to_usd/usd_utils.py` around lines 70 - 71, The current
validation does shape[1] checks on points and normals which will raise
IndexError for 1D arrays; update the checks to first verify ndim==2 (e.g., use
points.ndim != 2 or points.shape[1] != 3) and similarly for normals
(normals.ndim != 2 or normals.shape[1] != 3) so that invalid dimensionality
produces the intended ValueError; modify the existing conditionals that
reference points.shape[1] and normals.shape[1] to include the ndim guard and
keep the same ValueError messages.
Use ITK reader by default, except for NRRD which stores 4D data as images of vectors which is not available in ITK's python wrapping.
Summary by CodeRabbit
New Features
Enhancements
Documentation