ENH: Remove unreliable netgen meshing; fix py launcher and CT auto-convert#86
Merged
Conversation
…nvert Remove netgen-based tetrahedral volume meshing from the image-to-VTK workflow. netgen crashed on degenerate triangles produced by PyVista's clean() defaults, and — once that was fixed — proved to hang indefinitely on self-intersecting/non-manifold surfaces from real segmented anatomy (e.g. lung airways) rather than failing fast like well-formed surfaces do. Extensive testing showed no practical preprocessing (image padding, fill_holes, morphological closing) reliably prevents this, and fill_holes() itself carries a documented segfault risk. Rather than paper over an unreliable feature, drop it: ContourTools.extract_mesh/save_meshes/save_combined_mesh are gone, WorkflowConvertImageToVTK.process() no longer takes mesh_target_reduction and its result dict drops the 'meshes' key (breaking API change), the CLI drops --mesh-target-reduction and VTU output, and netgen-mesher is removed from pyproject.toml. Docs and tests updated accordingly. Fix the Windows `py` launcher resolving the wrong Python interpreter: a `#!/usr/bin/env python` shebang as line 1 makes py.exe search PATH for a bare `python` instead of using the active venv, breaking `physiotwin4d` imports. The shebang was leftover jupytext boilerplate with no functional purpose (not executable-bit set, bypassed by installed console-script entry points and by pytest's sys.executable-based invocation). Removed from all 64 affected files repo-wide. DataDownloadTools.DownloadSlicerHeartCTData now also splits the downloaded Slicer-Heart-CT 4D sequence into per-frame slice_???.mha volumes via ConvertImage4DTo3D, skipping the split once those files exist. Callers (tests, experiments, tutorials) no longer need a separate conversion step.
WalkthroughThe PR removes volumetric mesh generation and Netgen metadata, centralizes 4D dataset splitting, updates tutorials and documentation for surface-only outputs, changes statistical-model input discovery, and removes Python shebangs across scripts and tests. ChangesSurface conversion and data pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as convert_image_to_vtk
participant Workflow as WorkflowConvertImageToVTK
participant ContourTools
CLI->>Workflow: process image with output mode
Workflow->>ContourTools: extract anatomy and label surfaces
ContourTools-->>CLI: save VTP surfaces
sequenceDiagram
participant Downloader as DownloadSlicerHeartCTData
participant Converter as ConvertImage4DTo3D
participant Dataset as Slicer-Heart-CT
Downloader->>Dataset: download or reuse .seq.nrrd
Downloader->>Converter: split missing frames
Converter-->>Dataset: write slice_???.mha files
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
This PR removes the netgen-based tetrahedral volume meshing path from the image-to-VTK workflow/CLI (favoring surface-only outputs), eliminates #!/usr/bin/env python shebangs that interfere with Windows py.exe venv resolution, and updates Slicer-Heart-CT downloading to automatically split the 4D sequence into per-phase slice_???.mha volumes (with corresponding updates to tests/docs/tutorials).
Changes:
- Removed netgen tetrahedral meshing APIs (
ContourTools.extract_mesh/save_meshes/save_combined_mesh) and dropped mesh-related workflow/CLI parameters and outputs. - Removed
#!/usr/bin/env pythonshebangs repo-wide to prevent the Windows launcher from selecting the wrong interpreter. - Updated
DataDownloadTools.DownloadSlicerHeartCTDatato split.seq.nrrdintoslice_???.mhaframes automatically and updated callers/docs accordingly.
Reviewed changes
Copilot reviewed 80 out of 80 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/setup_feature_worktree.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| utils/generate_api_map.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| utils/ai_agent_github_reviews.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tutorials/tutorial_03_create_statistical_model.py | Updated KCL-Heart-Model inputs to .vtk layout and reference naming. |
| tutorials/tutorial_02_ct_to_vtk.py | Updated tutorial to surface-only output (no VTU mesh). |
| tests/test_vtk_to_usd_library.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_tutorials.py | Updated tutorial expectations (surface-only output). |
| tests/test_transform_tools.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_segment_heart_simpleware.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_segment_heart_simpleware_trimmed_branches.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_segment_chest_total_segmentator.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_register_time_series_images.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_register_images_icon.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_register_images_greedy.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_register_images_ants.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_labelmap_tools.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_image_tools.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_experiments.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_download_heart_data.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_convert_vtk_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_convert_image_4d_to_3d.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/test_contour_tools.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| tests/conftest.py | Updated fixtures to rely on auto-splitting Slicer-Heart-CT into slice_???.mha. |
| statistics.md | Removed netgen from dependency summary list. |
| src/physiotwin4d/workflow_convert_image_to_vtk.py | Dropped volume mesh extraction/output; surface-only workflow API. |
| src/physiotwin4d/data_download_tools.py | Auto-split Slicer-Heart-CT into per-phase .mha frames on download. |
| src/physiotwin4d/contour_tools.py | Removed netgen meshing and VTU save helpers; surface utilities remain. |
| src/physiotwin4d/cli/visualize_pca_modes.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/reconstruct_highres_4d_ct.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/fit_statistical_model_to_patient.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/download_data.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/create_statistical_model.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/convert_vtk_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/convert_image_to_vtk.py | Updated CLI help/behavior for surface-only export; removed mesh args/outputs. |
| src/physiotwin4d/cli/convert_image_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| src/physiotwin4d/cli/convert_image_4d_to_3d.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| README.md | Removed netgen mention and updated optional-license note accordingly. |
| pyproject.toml | Removed netgen-mesher dependency and mypy ignore entries. |
| experiments/Reconstruct4DCT/reconstruct_4d_ct.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Reconstruct4DCT/reconstruct_4d_ct_class.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-VesselsAirways/0-GenData.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/Experiment_SubSurfaceScatter.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/Experiment_SegReg.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/Experiment_CombineModels.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/Experiment_ArrangeOnStage.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/2-paint_dirlab_models.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/1-make_dirlab_models.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-VTKSeries_To_USD/1-heart_vtkseries_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.py | Simplified to rely on DownloadSlicerHeartCTData auto-splitting. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_registration_pca.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Simpleware_Segmentation/simpleware_heart_segmentation.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-GatedCT_To_USD/4-merge_dynamic_and_static_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-GatedCT_To_USD/2-generate_segmentation.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-GatedCT_To_USD/1-register_images.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py | Simplified to rely on DownloadSlicerHeartCTData auto-splitting. |
| experiments/Heart-Create_Statistical_Model/5-compute_pca_model.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Create_Statistical_Model/4-surfaces_aligned_correspond_to_pca_inputs.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/DisplacementField_To_USD/displacement_field_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Convert_VTK_To_USD/convert_chop_heart_vtk_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| experiments/Colormap-VTK_To_USD/colormap_vtk_to_usd.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| docs/tutorials.rst | Updated tutorial narrative/examples for surface-only workflow. |
| docs/developer/architecture.rst | Updated workflow description to surface-only outputs. |
| docs/cli_scripts/overview.rst | Updated CLI description to surface-only outputs. |
| docs/cli_scripts/download_data.rst | Documented auto-splitting Slicer-Heart-CT into slice_???.mha. |
| docs/architecture.rst | Updated architecture docs for surface-only image-to-VTK workflow. |
| docs/api/workflows.rst | Updated workflow listing/description for surface-only export. |
| data/Slicer-Heart-CT/README.md | Documented auto-splitting downloaded .seq.nrrd into slices. |
| data/DirLab-4DCT/Convert4DCTToMHD.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
| .github/scripts/build_dashboard.py | Removed shebang to avoid Windows py.exe interpreter mis-resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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/conftest.py (1)
453-462: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocstring omits image shape/axis order.
Path instructions for
tests/**/*.pyrequire every test docstring to state image shape and axis order (e.g.LPS world frame). The updated docstring only says "return time points" without noting these are per-frame 3D ITK volumes in LPS world space.As per path instructions, "State image shape and axis order in every test docstring, for example
shape (X, Y, Z, T) = (64, 64, 32, 1), LPS world frame."🤖 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/conftest.py` around lines 453 - 462, Update the test_images docstring to state that the returned time points are per-frame 3D ITK volumes and explicitly identify their axis order/world frame as LPS, while preserving the existing resampling description.Source: Path instructions
🧹 Nitpick comments (1)
docs/cli_scripts/overview.rst (1)
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "meshes/" entry in Output Organization example.
Line 57 was updated to describe surface-only output, but the "Output Organization" example directory tree at line 116 still lists a
meshes/folder with# VTK mesh files, which is now inconsistent with the removal of volumetric mesh generation from this workflow.📝 Suggested fix
output_directory/ ├── intermediate/ # Segmentations, registrations - └── meshes/ # VTK mesh files + ├── surfaces/ # VTK surface (VTP) files └── usd/ # Final USD files for OmniverseAlso applies to: 112-117
🤖 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` around lines 56 - 57, Update the Output Organization example associated with physiotwin4d-convert-image-to-vtk to remove the stale meshes/ directory and its VTK mesh files comment, keeping the directory tree consistent with surface-only anatomy-group VTK output.
🤖 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 `@data/Slicer-Heart-CT/README.md`:
- Around line 20-33: Update the directory-structure fenced code block in the
README to include the text language identifier, preserving its existing contents
and formatting.
In `@src/physiotwin4d/data_download_tools.py`:
- Around line 94-103: Make the Slicer-Heart conversion in the surrounding
download flow atomic: replace the single-file existence check and direct save
into data_dir with a temporary-directory conversion using ConvertImage4DTo3D,
then atomically move or rename the completed output into place only after
save_3d_images succeeds. Ensure failed or interrupted conversions leave no
partial slice_???.mha files and concurrent callers cannot treat an incomplete
split as valid, following the existing download/extract helper pattern.
In `@tutorials/tutorial_03_create_statistical_model.py`:
- Around line 73-76: Update the fallback assignment in the sample-file discovery
flow to exclude the reference mesh file before populating sample_files, while
preserving the existing input_meshes/*.vtk behavior. Use the reference mesh
filename already established by the tutorial’s PCA setup so it cannot be
included as both alignment target and training sample.
---
Outside diff comments:
In `@tests/conftest.py`:
- Around line 453-462: Update the test_images docstring to state that the
returned time points are per-frame 3D ITK volumes and explicitly identify their
axis order/world frame as LPS, while preserving the existing resampling
description.
---
Nitpick comments:
In `@docs/cli_scripts/overview.rst`:
- Around line 56-57: Update the Output Organization example associated with
physiotwin4d-convert-image-to-vtk to remove the stale meshes/ directory and its
VTK mesh files comment, keeping the directory tree consistent with surface-only
anatomy-group VTK output.
🪄 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: 86ceb226-5a02-4a10-81eb-25e73ee2e419
📒 Files selected for processing (80)
.github/scripts/build_dashboard.pyREADME.mddata/DirLab-4DCT/Convert4DCTToMHD.pydata/Slicer-Heart-CT/README.mddocs/api/workflows.rstdocs/architecture.rstdocs/cli_scripts/download_data.rstdocs/cli_scripts/overview.rstdocs/developer/architecture.rstdocs/tutorials.rstexperiments/Colormap-VTK_To_USD/colormap_vtk_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_heart_vtk_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.pyexperiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.pyexperiments/DisplacementField_To_USD/displacement_field_to_usd.pyexperiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.pyexperiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.pyexperiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.pyexperiments/Heart-Create_Statistical_Model/4-surfaces_aligned_correspond_to_pca_inputs.pyexperiments/Heart-Create_Statistical_Model/5-compute_pca_model.pyexperiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-GatedCT_To_USD/1-register_images.pyexperiments/Heart-GatedCT_To_USD/2-generate_segmentation.pyexperiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.pyexperiments/Heart-GatedCT_To_USD/4-merge_dynamic_and_static_usd.pyexperiments/Heart-Simpleware_Segmentation/simpleware_heart_segmentation.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_registration_pca.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.pyexperiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-VTKSeries_To_USD/1-heart_vtkseries_to_usd.pyexperiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.pyexperiments/Lung-GatedCT_To_USD/1-make_dirlab_models.pyexperiments/Lung-GatedCT_To_USD/2-paint_dirlab_models.pyexperiments/Lung-GatedCT_To_USD/Experiment_ArrangeOnStage.pyexperiments/Lung-GatedCT_To_USD/Experiment_CombineModels.pyexperiments/Lung-GatedCT_To_USD/Experiment_SegReg.pyexperiments/Lung-GatedCT_To_USD/Experiment_SubSurfaceScatter.pyexperiments/Lung-VesselsAirways/0-GenData.pyexperiments/Reconstruct4DCT/reconstruct_4d_ct.pyexperiments/Reconstruct4DCT/reconstruct_4d_ct_class.pypyproject.tomlsrc/physiotwin4d/cli/convert_image_4d_to_3d.pysrc/physiotwin4d/cli/convert_image_to_usd.pysrc/physiotwin4d/cli/convert_image_to_vtk.pysrc/physiotwin4d/cli/convert_vtk_to_usd.pysrc/physiotwin4d/cli/create_statistical_model.pysrc/physiotwin4d/cli/download_data.pysrc/physiotwin4d/cli/fit_statistical_model_to_patient.pysrc/physiotwin4d/cli/reconstruct_highres_4d_ct.pysrc/physiotwin4d/cli/visualize_pca_modes.pysrc/physiotwin4d/contour_tools.pysrc/physiotwin4d/data_download_tools.pysrc/physiotwin4d/workflow_convert_image_to_vtk.pystatistics.mdtests/conftest.pytests/test_contour_tools.pytests/test_convert_image_4d_to_3d.pytests/test_convert_vtk_to_usd.pytests/test_download_heart_data.pytests/test_experiments.pytests/test_image_tools.pytests/test_labelmap_tools.pytests/test_register_images_ants.pytests/test_register_images_greedy.pytests/test_register_images_icon.pytests/test_register_time_series_images.pytests/test_segment_chest_total_segmentator.pytests/test_segment_heart_simpleware.pytests/test_segment_heart_simpleware_trimmed_branches.pytests/test_transform_tools.pytests/test_tutorials.pytests/test_vtk_to_usd_library.pytutorials/tutorial_02_ct_to_vtk.pytutorials/tutorial_03_create_statistical_model.pyutils/ai_agent_github_reviews.pyutils/generate_api_map.pyutils/setup_feature_worktree.py
💤 Files with no reviewable changes (62)
- experiments/Lung-GatedCT_To_USD/Experiment_ArrangeOnStage.py
- tests/test_image_tools.py
- tests/test_experiments.py
- experiments/Heart-Simpleware_Segmentation/simpleware_heart_segmentation.py
- experiments/Heart-GatedCT_To_USD/2-generate_segmentation.py
- experiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.py
- experiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.py
- experiments/Lung-VesselsAirways/0-GenData.py
- experiments/Lung-GatedCT_To_USD/Experiment_SubSurfaceScatter.py
- utils/setup_feature_worktree.py
- src/physiotwin4d/cli/create_statistical_model.py
- experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.py
- experiments/DisplacementField_To_USD/displacement_field_to_usd.py
- tests/test_vtk_to_usd_library.py
- experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_registration_pca.py
- experiments/Lung-GatedCT_To_USD/Experiment_SegReg.py
- experiments/Reconstruct4DCT/reconstruct_4d_ct_class.py
- src/physiotwin4d/cli/reconstruct_highres_4d_ct.py
- tests/test_segment_chest_total_segmentator.py
- experiments/Convert_VTK_To_USD/convert_chop_heart_vtk_to_usd.py
- src/physiotwin4d/cli/convert_vtk_to_usd.py
- experiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.py
- experiments/Lung-GatedCT_To_USD/2-paint_dirlab_models.py
- experiments/Heart-GatedCT_To_USD/1-register_images.py
- experiments/Heart-VTKSeries_To_USD/1-heart_vtkseries_to_usd.py
- src/physiotwin4d/cli/convert_image_4d_to_3d.py
- utils/generate_api_map.py
- experiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.py
- experiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.py
- src/physiotwin4d/cli/fit_statistical_model_to_patient.py
- src/physiotwin4d/cli/visualize_pca_modes.py
- tests/test_register_images_ants.py
- tests/test_register_images_greedy.py
- experiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.py
- tests/test_convert_image_4d_to_3d.py
- experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py
- tests/test_register_time_series_images.py
- tests/test_download_heart_data.py
- tests/test_register_images_icon.py
- experiments/Heart-Create_Statistical_Model/5-compute_pca_model.py
- tests/test_transform_tools.py
- tests/test_convert_vtk_to_usd.py
- experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py
- experiments/Lung-GatedCT_To_USD/Experiment_CombineModels.py
- tests/test_contour_tools.py
- experiments/Lung-GatedCT_To_USD/1-make_dirlab_models.py
- experiments/Colormap-VTK_To_USD/colormap_vtk_to_usd.py
- tests/test_segment_heart_simpleware.py
- tests/test_tutorials.py
- tests/test_labelmap_tools.py
- experiments/Heart-GatedCT_To_USD/4-merge_dynamic_and_static_usd.py
- data/DirLab-4DCT/Convert4DCTToMHD.py
- experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.py
- src/physiotwin4d/cli/download_data.py
- experiments/Reconstruct4DCT/reconstruct_4d_ct.py
- utils/ai_agent_github_reviews.py
- experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.py
- tests/test_segment_heart_simpleware_trimmed_branches.py
- experiments/Heart-Create_Statistical_Model/4-surfaces_aligned_correspond_to_pca_inputs.py
- .github/scripts/build_dashboard.py
- src/physiotwin4d/cli/convert_image_to_usd.py
- pyproject.toml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/tutorials.rst (1)
213-226: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCode example doesn't match the described
extract_label_surfacesbehavior.The snippet omits
extract_label_surfaces=SAVE_LABEL_SURFACES, but the prose right below states that enabling label surfaces "passesextract_label_surfaces=True" — and the actual tutorial script does pass it. As written, copying this snippet verbatim won't reproduce the documented behavior.📝 Suggested fix
result = workflow.process( input_image=ct_image, surface_target_reduction=0.5, + extract_label_surfaces=SAVE_LABEL_SURFACES, )🤖 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/tutorials.rst` around lines 213 - 226, Update the `workflow.process` example in `docs/tutorials.rst` to pass `extract_label_surfaces=SAVE_LABEL_SURFACES`, matching the documented behavior and tutorial script while preserving the existing arguments.
🧹 Nitpick comments (1)
src/physiotwin4d/workflow_convert_image_to_vtk.py (1)
190-209: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid re-converting the full labelmap array on every label.
_extract_label_surfacecallsitk.GetArrayFromImage(labelmap_image)internally, andprocess()invokes it once per(group, label_id)pair against the sameseg_result["labelmap"]object. For taxonomies with many labels across groups, this repeats a full-volume ITK→NumPy copy on every label instead of once, adding avoidable I/O-bound overhead whenextract_label_surfaces=True.♻️ Cache the labelmap array once and reuse it across labels
def _extract_label_surface( - self, labelmap_image: Any, label_id: int + self, labelmap_image: Any, label_id: int, labelmap_arr: Optional[Any] = None ) -> Optional[pv.PolyData]: """Extract a smoothed triangulated surface for one individual label. ... """ - arr = itk.GetArrayFromImage(labelmap_image) + arr = labelmap_arr if labelmap_arr is not None else itk.GetArrayFromImage( + labelmap_image + ) label_mask_arr = (arr == label_id).astype(np.uint8) if int(label_mask_arr.sum()) == 0: return None label_mask = itk.GetImageFromArray(label_mask_arr) label_mask.CopyInformation(labelmap_image) return self._contour_tools.extract_contours(label_mask)+ labelmap_arr_cache: Optional[Any] = None for group in groups_to_process: ... if extract_label_surfaces: self.log_info(" Extracting label surfaces for: %s", group) + if labelmap_arr_cache is None: + labelmap_arr_cache = itk.GetArrayFromImage(seg_result["labelmap"]) for label_id, label_name in zip(label_ids, label_names, strict=True): label_surface = self._extract_label_surface( - seg_result["labelmap"], label_id + seg_result["labelmap"], label_id, labelmap_arr_cache )Also applies to: 310-325
🤖 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/physiotwin4d/workflow_convert_image_to_vtk.py` around lines 190 - 209, Cache the NumPy conversion of seg_result["labelmap"] once in process() and pass or reuse that array when handling each (group, label_id) pair. Update _extract_label_surface to consume the cached array while preserving mask creation, metadata copying, empty-label handling, and contour extraction behavior.
🤖 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 `@tests/test_workflow_convert_image_to_vtk.py`:
- Around line 37-58: Update the test_extract_label_surface_isolates_single_label
docstring to state that its input is synthetic and document the (10, 10, 10)
image shape and the array’s axis order. Preserve the existing
regression-behavior description and test logic.
---
Outside diff comments:
In `@docs/tutorials.rst`:
- Around line 213-226: Update the `workflow.process` example in
`docs/tutorials.rst` to pass `extract_label_surfaces=SAVE_LABEL_SURFACES`,
matching the documented behavior and tutorial script while preserving the
existing arguments.
---
Nitpick comments:
In `@src/physiotwin4d/workflow_convert_image_to_vtk.py`:
- Around line 190-209: Cache the NumPy conversion of seg_result["labelmap"] once
in process() and pass or reuse that array when handling each (group, label_id)
pair. Update _extract_label_surface to consume the cached array while preserving
mask creation, metadata copying, empty-label handling, and contour extraction
behavior.
🪄 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: c00f20b2-86ea-4d30-80cc-eda0c3bc3612
📒 Files selected for processing (10)
data/Slicer-Heart-CT/README.mddocs/tutorials.rstsrc/physiotwin4d/cli/convert_image_to_vtk.pysrc/physiotwin4d/data_download_tools.pysrc/physiotwin4d/workflow_convert_image_to_vtk.pysrc/physiotwin4d/workflow_fit_statistical_model_to_patient.pytests/test_workflow_convert_image_to_vtk.pytests/test_workflow_fit_statistical_model_to_patient.pytutorials/tutorial_02_ct_to_vtk.pytutorials/tutorial_03_create_statistical_model.py
🚧 Files skipped from review as they are similar to previous changes (3)
- data/Slicer-Heart-CT/README.md
- src/physiotwin4d/data_download_tools.py
- tutorials/tutorial_03_create_statistical_model.py
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 `@tests/test_tutorials.py`:
- Around line 166-169: Update the tutorial 04 dataset guard in the relevant test
setup to require both kcl_dir / "average_mesh.vtk" and kcl_dir / "pca_mean.vtu"
before running. Preserve the existing pytest.skip message and behavior when
either file is missing.
🪄 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: d3814f5a-c90d-4bc7-b2c4-3eec7f13c13e
📒 Files selected for processing (3)
src/physiotwin4d/workflow_convert_image_to_vtk.pytests/test_tutorials.pytests/test_workflow_convert_image_to_vtk.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_workflow_convert_image_to_vtk.py
- src/physiotwin4d/workflow_convert_image_to_vtk.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove netgen-based tetrahedral volume meshing from the image-to-VTK workflow. netgen crashed on degenerate triangles produced by PyVista's clean() defaults, and — once that was fixed — proved to hang indefinitely on self-intersecting/non-manifold surfaces from real segmented anatomy (e.g. lung airways) rather than failing fast like well-formed surfaces do. Extensive testing showed no practical preprocessing (image padding, fill_holes, morphological closing) reliably prevents this, and fill_holes() itself carries a documented segfault risk. Rather than paper over an unreliable feature, drop it: ContourTools.extract_mesh/save_meshes/save_combined_mesh are gone, WorkflowConvertImageToVTK.process() no longer takes mesh_target_reduction and its result dict drops the 'meshes' key (breaking API change), the CLI drops --mesh-target-reduction and VTU output, and netgen-mesher is removed from pyproject.toml. Docs and tests updated accordingly.
Fix the Windows
pylauncher resolving the wrong Python interpreter: a#!/usr/bin/env pythonshebang as line 1 makes py.exe search PATH for a barepythoninstead of using the active venv, breakingphysiotwin4dimports. The shebang was leftover jupytext boilerplate with no functional purpose (not executable-bit set, bypassed by installed console-script entry points and by pytest's sys.executable-based invocation). Removed from all 64 affected files repo-wide.DataDownloadTools.DownloadSlicerHeartCTData now also splits the downloaded Slicer-Heart-CT 4D sequence into per-frame slice_???.mha volumes via ConvertImage4DTo3D, skipping the split once those files exist. Callers (tests, experiments, tutorials) no longer need a separate conversion step.
Summary by CodeRabbit
slice_*.mha), and repeated runs reuse existing outputs and skip completed splitting.