fix: replace absolute intra-package imports with relative imports#67
Merged
Conversation
All submodules imported `from physiomotion4d.X import Y` (or `from physiomotion4d import Y`), which causes a circular import error at package initialization time — `__init__.py` imports each submodule while the submodules try to reach back into the partially-initialized package. This fails consistently regardless of install method (pip install ., editable install, or PyPI wheel). Replaced all 34 affected files with PEP 328 relative imports (`from .X import Y` for package submodules, `from ..X import Y` for the cli/ sub-package). Docstring examples left unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces ANTs/SyN distance-map registration with a Greedy affine + ICON deformable pipeline, updates RegisterModelsDistanceMaps API and compositions, removes Stage-3 ICON toggle from workflow calls, revises docs/notebooks/READMEs, and converts many absolute imports to package-relative imports. ChangesGreedy + ICON Registration
Package Import Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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)
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 aims to eliminate circular imports during physiomotion4d package initialization by switching intra-package imports from absolute (from physiomotion4d...) to PEP 328 relative imports (from . ..., from .. ...). In addition, it updates the mask-based model registration pipeline and accompanying docs/experiments to reflect a Greedy+ICON workflow.
Changes:
- Replace absolute intra-package imports with relative imports across modules and CLI subpackage.
- Refactor
RegisterModelsDistanceMapsto use Greedy (rigid/affine) + ICON (deformable) and update call sites accordingly. - Update documentation/experiments to reflect the new registration pipeline and changed workflow API.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tutorials/tutorial_04_fit_statistical_model_to_patient.py | Minor tutorial comment tweak around Windows multiprocessing guard. |
| src/physiomotion4d/workflow_reconstruct_highres_4d_ct.py | Switch workflow imports to relative to avoid package init cycles. |
| src/physiomotion4d/workflow_fit_statistical_model_to_patient.py | Relative imports + mask-to-mask stage API/call updates. |
| src/physiomotion4d/workflow_fine_tune_icon_registration.py | Switch workflow imports to relative. |
| src/physiomotion4d/workflow_create_statistical_model.py | Relative imports + update usage of distance-map registrar API. |
| src/physiomotion4d/workflow_convert_vtk_to_usd.py | Switch workflow imports to relative. |
| src/physiomotion4d/workflow_convert_image_to_vtk.py | Relative imports, including lazy-import segmenter paths. |
| src/physiomotion4d/workflow_convert_image_to_usd.py | Replace from physiomotion4d import ... with local relative imports. |
| src/physiomotion4d/usd_tools.py | Switch imports to relative. |
| src/physiomotion4d/usd_anatomy_tools.py | Switch imports to relative. |
| src/physiomotion4d/transform_tools.py | Switch imports to relative. |
| src/physiomotion4d/test_tools.py | Switch imports to relative, including a local import inside helper. |
| src/physiomotion4d/segment_heart_simpleware.py | Switch base-class import to relative. |
| src/physiomotion4d/segment_chest_total_segmentator.py | Switch base-class import to relative. |
| src/physiomotion4d/segment_anatomy_base.py | Switch internal imports to relative. |
| src/physiomotion4d/register_time_series_images.py | Switch internal imports to relative. |
| src/physiomotion4d/register_models_pca.py | Switch internal imports to relative. |
| src/physiomotion4d/register_models_icp.py | Switch internal imports to relative. |
| src/physiomotion4d/register_models_icp_itk.py | Switch internal imports to relative. |
| src/physiomotion4d/register_models_distance_maps.py | Switch internal imports to relative; refactor implementation/docs to Greedy+ICON and update API. |
| src/physiomotion4d/register_images_icon.py | Switch internal imports to relative. |
| src/physiomotion4d/register_images_greedy.py | Switch internal imports to relative (including local imports). |
| src/physiomotion4d/register_images_base.py | Switch internal imports to relative. |
| src/physiomotion4d/register_images_ants.py | Switch internal imports to relative (including local imports). |
| src/physiomotion4d/landmark_tools.py | Switch internal imports to relative. |
| src/physiomotion4d/labelmap_tools.py | Switch internal imports to relative. |
| src/physiomotion4d/image_tools.py | Switch internal imports to relative. |
| src/physiomotion4d/convert_image_4d_to_3d.py | Switch internal imports to relative. |
| src/physiomotion4d/contour_tools.py | Switch internal imports to relative. |
| src/physiomotion4d/cli/reconstruct_highres_4d_ct.py | Change CLI import to relative-from-parent package. |
| src/physiomotion4d/cli/fit_statistical_model_to_patient.py | Change CLI import to relative-from-parent package. |
| src/physiomotion4d/cli/download_data.py | Change CLI import to relative-from-parent package module. |
| src/physiomotion4d/cli/create_statistical_model.py | Change CLI import to relative-from-parent package. |
| src/physiomotion4d/cli/convert_vtk_to_usd.py | Change CLI imports to relative-from-parent package. |
| src/physiomotion4d/cli/convert_image_to_vtk.py | Change CLI import to relative-from-parent package. |
| src/physiomotion4d/cli/convert_image_to_usd.py | Change CLI import to relative-from-parent package. |
| src/physiomotion4d/cli/convert_image_4d_to_3d.py | Change CLI import to relative-from-parent package. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.py | Update call to mask-to-mask registration after API change. |
| experiments/Heart-Create_Statistical_Model/README.md | Update narrative/docs from ANTs SyN to Greedy+ICON pipeline. |
| experiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.py | Update experiment steps and registrar call to new API. |
| docs/cli_scripts/fit_statistical_model_to_patient.rst | Update CLI documentation for ICON refinement semantics and mask-to-mask stage description. |
Comments suppressed due to low confidence (1)
src/physiomotion4d/register_models_distance_maps.py:255
- The
register()docstring says the return dict contains'moving_model', but the method actually returns'registered_model'. This makes the API documentation inaccurate for callers reading the docstring.
Returns:
Dictionary containing:
- 'moving_model': Aligned moving model (PyVista PolyData)
- 'forward_transform': Moving→fixed transform (ITK CompositeTransform)
- 'inverse_transform': Fixed→moving transform (ITK CompositeTransform)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
314
to
330
| # Pre-align moving mask with the Greedy affine result | ||
| moving_mask_affine_transformed = self.transform_tools.transform_image( | ||
| self.moving_mask_image, | ||
| forward_transform_ANTS, | ||
| forward_transform_Greedy, | ||
| self.reference_image, | ||
| interpolation_method="linear", | ||
| ) | ||
|
|
||
| # Configure ICON | ||
| # Configure and run ICON | ||
| self.registrar_ICON.set_number_of_iterations(icon_iterations) | ||
| self.registrar_ICON.set_fixed_image(self.fixed_mask_image) | ||
| self.registrar_ICON.set_fixed_mask(self.fixed_mask_roi_image) | ||
|
|
||
| # ICON registration | ||
| result_ICON = self.registrar_ICON.register( | ||
| moving_image=moving_mask_ANTS_transformed, | ||
| moving_image=moving_mask_affine_transformed, | ||
| moving_mask=self.moving_mask_roi_image, | ||
| ) |
Comment on lines
1
to
+10
| """Mask-based model-to-model registration for anatomical models. | ||
|
|
||
| This module provides the RegisterModelsDistanceMaps class for aligning anatomical | ||
| models using mask-based deformable registration. The workflow includes: | ||
| 1. Generate binary masks from moving and fixed models | ||
| 2. Generate ROI masks with dilation | ||
| 4. Progressive registration stages: | ||
| - rigid: ANTs rigid registration | ||
| - affine: ANTs rigid → affine registration | ||
| - deformable: ANTs rigid → affine → deformable (SyN) registration | ||
| 5. Optional ICON refinement at end | ||
| 3. Progressive registration stages: | ||
| - rigid: Greedy rigid registration | ||
| - affine: Greedy affine registration | ||
| - deformable: Greedy affine → ICON deformable registration |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/register_models_distance_maps.py`:
- Around line 315-330: The moving ROI mask is still the untransformed mask
(moving_mask_roi_image) when calling registrar_ICON.register, causing a space
mismatch; transform the ROI mask with the same affine pre-alignment (use
transform_tools.transform_image with forward_transform_Greedy and the same
reference_image) — use a mask-appropriate interpolation (e.g., nearest) — and
pass that transformed ROI (instead of moving_mask_roi_image) into
registrar_ICON.register; update references around
moving_mask_affine_transformed, moving_mask_roi_image,
transform_tools.transform_image, forward_transform_Greedy, and
registrar_ICON.register.
🪄 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: f8685517-7da6-4a16-bc53-563718b44de2
📒 Files selected for processing (41)
docs/cli_scripts/fit_statistical_model_to_patient.rstexperiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.pyexperiments/Heart-Create_Statistical_Model/README.mdexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.pysrc/physiomotion4d/cli/convert_image_4d_to_3d.pysrc/physiomotion4d/cli/convert_image_to_usd.pysrc/physiomotion4d/cli/convert_image_to_vtk.pysrc/physiomotion4d/cli/convert_vtk_to_usd.pysrc/physiomotion4d/cli/create_statistical_model.pysrc/physiomotion4d/cli/download_data.pysrc/physiomotion4d/cli/fit_statistical_model_to_patient.pysrc/physiomotion4d/cli/reconstruct_highres_4d_ct.pysrc/physiomotion4d/contour_tools.pysrc/physiomotion4d/convert_image_4d_to_3d.pysrc/physiomotion4d/image_tools.pysrc/physiomotion4d/labelmap_tools.pysrc/physiomotion4d/landmark_tools.pysrc/physiomotion4d/register_images_ants.pysrc/physiomotion4d/register_images_base.pysrc/physiomotion4d/register_images_greedy.pysrc/physiomotion4d/register_images_icon.pysrc/physiomotion4d/register_models_distance_maps.pysrc/physiomotion4d/register_models_icp.pysrc/physiomotion4d/register_models_icp_itk.pysrc/physiomotion4d/register_models_pca.pysrc/physiomotion4d/register_time_series_images.pysrc/physiomotion4d/segment_anatomy_base.pysrc/physiomotion4d/segment_chest_total_segmentator.pysrc/physiomotion4d/segment_heart_simpleware.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/transform_tools.pysrc/physiomotion4d/usd_anatomy_tools.pysrc/physiomotion4d/usd_tools.pysrc/physiomotion4d/workflow_convert_image_to_usd.pysrc/physiomotion4d/workflow_convert_image_to_vtk.pysrc/physiomotion4d/workflow_convert_vtk_to_usd.pysrc/physiomotion4d/workflow_create_statistical_model.pysrc/physiomotion4d/workflow_fine_tune_icon_registration.pysrc/physiomotion4d/workflow_fit_statistical_model_to_patient.pysrc/physiomotion4d/workflow_reconstruct_highres_4d_ct.pytutorials/tutorial_04_fit_statistical_model_to_patient.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
+ Coverage 32.90% 32.92% +0.01%
==========================================
Files 53 53
Lines 7229 7229
==========================================
+ Hits 2379 2380 +1
+ Misses 4850 4849 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/physiomotion4d/register_models_distance_maps.py:255
- The register() docstring lists a 'moving_model' return key, but the implementation returns 'registered_model'. This is a public-facing API mismatch and can lead to KeyError in user code that follows the docs.
Returns:
Dictionary containing:
- 'moving_model': Aligned moving model (PyVista PolyData)
- 'forward_transform': Moving→fixed transform (ITK CompositeTransform)
- 'inverse_transform': Fixed→moving transform (ITK CompositeTransform)
Comment on lines
3
to
+7
| This module provides the RegisterModelsDistanceMaps class for aligning anatomical | ||
| models using mask-based deformable registration. The workflow includes: | ||
| 1. Generate binary masks from moving and fixed models | ||
| 2. Generate ROI masks with dilation | ||
| 4. Progressive registration stages: | ||
| - rigid: ANTs rigid registration | ||
| - affine: ANTs rigid → affine registration | ||
| - deformable: ANTs rigid → affine → deformable (SyN) registration | ||
| 5. Optional ICON refinement at end | ||
| 3. Progressive registration stages: |
Comment on lines
225
to
229
| def register( | ||
| self, | ||
| transform_type: str = "Deformable", | ||
| use_ICON: bool = False, | ||
| icon_iterations: int = 50, | ||
| ) -> dict: |
Comment on lines
83
to
87
| **Advantages:** | ||
| - Fully automated (no manual parameter tuning) | ||
| - Handles complex topologies naturally | ||
| - Diffeomorphic guarantees smooth, invertible deformations | ||
| - Composed Greedy + ICON transforms provide smooth, invertible deformation fields | ||
| - Integrates seamlessly with medical imaging pipelines |
Comment on lines
+338
to
+342
| # - The `RegisterModelsDistanceMaps` class uses a two-stage pipeline: | ||
| # 1. **Greedy affine** registration (fast CPU-based alignment) | ||
| # 2. **ICON deformable** registration on the affine-pre-aligned masks (deep learning) | ||
| # - The `roi_dilation_mm` parameter controls the dilation of the ROI mask (default 20mm) | ||
| # - SyN registration provides smooth, invertible deformation fields for anatomical correspondence | ||
| # - Composed Greedy + ICON transforms provide smooth, invertible deformation fields for anatomical correspondence |
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.
All submodules imported
from physiomotion4d.X import Y(orfrom physiomotion4d import Y), which causes a circular import error at package initialization time —__init__.pyimports each submodule while the submodules try to reach back into the partially-initialized package. This fails consistently regardless of install method (pip install ., editable install, or PyPI wheel).Replaced all 34 affected files with PEP 328 relative imports (
from .X import Yfor package submodules,from ..X import Yfor the cli/ sub-package). Docstring examples left unchanged.Summary by CodeRabbit
Documentation
Refactor