Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoved VISTA‑3D implementations, NIM integration, ensemble module and their tests/docs; updated workflow and exports to exclude VISTA‑3D; added Codecov tokens to CI uploads; adjusted many docs, test fixtures/typing and download error behavior; replaced multiple mesh extract_surface calls; updated .gitignore entries. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 tightens up the project’s testing and CI plumbing by improving external test-data download behavior in CI, ensuring Codecov uploads include the repo token, and aligning documentation badges with the nightly health workflow.
Changes:
- Update the TruncalValve test-data download URL and fail (instead of skip) on download HTTP errors when running in CI.
- Add
CODECOV_TOKENto Codecov upload steps in CI and slow-test workflows. - Update docs CI badge to point at the
nightly-health.ymlworkflow and refresh the generated API map.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/conftest.py |
Switches test-data download URL and changes CI behavior to fail on download HTTP errors. |
docs/index.rst |
Updates the CI badge to reflect the nightly health workflow. |
docs/API_MAP.md |
Regenerates API map line numbers after code changes. |
.github/workflows/test-slow.yml |
Adds Codecov token to coverage upload. |
.github/workflows/ci.yml |
Adds Codecov token to coverage uploads across unit/integration/GPU jobs. |
.claude/.gitignore |
Ignores local Claude settings file. |
💡 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/conftest.py`:
- Around line 297-305: The except block currently catches urllib.error.HTTPError
only, so network/DNS failures (urllib.error.URLError) bypass the CI/local
fail-or-skip logic; change the handler to catch urllib.error.URLError (or both
urllib.error.HTTPError and urllib.error.URLError) instead of only HTTPError so
that the same pytest.fail/pytest.skip behavior (using data_dir) runs for
network-level errors; update the except clause where urllib.error.HTTPError is
referenced to use urllib.error.URLError (or a tuple) and keep the existing msg,
pytest.fail and pytest.skip calls unchanged.
🪄 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: a5a7fc0d-8da7-480b-9770-0a4561357932
📒 Files selected for processing (6)
.claude/.gitignore.github/workflows/ci.yml.github/workflows/test-slow.ymldocs/API_MAP.mddocs/index.rsttests/conftest.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
=======================================
Coverage ? 13.91%
=======================================
Files ? 46
Lines ? 6412
Branches ? 0
=======================================
Hits ? 892
Misses ? 5520
Partials ? 0
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.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
294
to
+298
| urllib.request.urlretrieve(input_image_url, str(input_image_filename)) | ||
| print(f"Downloaded to {input_image_filename}") | ||
| except urllib.error.HTTPError as e: | ||
| pytest.skip( | ||
| f"Could not download test data: {e}. Please manually place TruncalValve_4DCT.seq.nrrd in {data_dir}" | ||
| except urllib.error.URLError as e: | ||
| msg = ( | ||
| f"Could not download test data: {e}. " |
There was a problem hiding this comment.
The except urllib.error.URLError clause will fail if triggered because urllib.error isn’t imported (only urllib.request is). Import urllib.error (or from urllib.error import URLError) so download failures are handled as intended.
Comment on lines
23
to
26
| **Segmentation Classes**: | ||
| * :doc:`base` - Base class for all segmentation methods | ||
| * :doc:`totalsegmentator` - TotalSegmentator implementation | ||
| * :doc:`vista3d` - VISTA-3D foundation model | ||
| * :doc:`vista3d_nim` - VISTA-3D NIM for cloud deployment | ||
| * :doc:`ensemble` - Ensemble segmentation |
There was a problem hiding this comment.
This PR removes the vista3d/vista3d_nim pages from the segmentation docs, but other docs in the repo still reference :doc:vista3d`` (e.g., docs/api/segmentation/base.rst and `docs/api/segmentation/totalsegmentator.rst`) and `docs/api/workflows.rst` mentions `segmentation_method="vista3d"`. Those references should be updated (or stub pages kept) to avoid broken Sphinx links.
Comment on lines
+263
to
270
| from physiomotion4d import SegmentChestTotalSegmentator | ||
| import itk | ||
|
|
||
| # Initialize VISTA-3D segmentation | ||
| segmenter = SegmentChestVista3D() | ||
| # Initialize TotalSegmentator segmentation | ||
| segmenter = SegmentChestTotalSegmentator() | ||
|
|
||
| # Load and segment image | ||
| image = itk.imread("chest_ct.nrrd") |
There was a problem hiding this comment.
In this example, SegmentChestTotalSegmentator.segment() returns a dict of masks (e.g., result['labelmap'], result['heart']). The snippet below currently assigns to masks = ... and then unpacks multiple values; after switching to TotalSegmentator the example should be updated to use the dict keys instead of tuple unpacking.
Comment on lines
+119
to
126
| from physiomotion4d import SegmentChestTotalSegmentator | ||
| import itk | ||
|
|
||
| # Initialize segmenter | ||
| segmenter = SegmentChestVista3D() | ||
| segmenter = SegmentChestTotalSegmentator() | ||
|
|
||
| # Load and segment image | ||
| image = itk.imread("chest_ct.nrrd") |
There was a problem hiding this comment.
SegmentChestTotalSegmentator.segment() returns a dict of masks, but this section’s code block unpacks the return value into multiple variables. After updating the import/constructor here, please also update the subsequent lines to use keys like result['heart'] / result['labelmap'] rather than tuple unpacking.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/developer/segmentation.rst (1)
104-128: Single-method ensemble example may be misleading.The ensemble documentation states it "combines multiple segmentation methods" (line 105), but the example uses only
methods=['totalsegmentator']. This inconsistency could confuse developers.Recommendation: Either update the feature description to note that it currently uses a single backend, or remove the
methodsparameter from the example sinceSegmentChestEnsemblenow directly inherits and delegates toSegmentChestTotalSegmentator.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer/segmentation.rst` around lines 104 - 128, The docs example is inconsistent: SegmentChestEnsemble claims to combine multiple methods but the example passes methods=['totalsegmentator']; update the documentation to remove or clarify the misleading parameter. Edit the example usage for SegmentChestEnsemble to either (A) remove the methods argument and show direct instantiation (since SegmentChestEnsemble delegates to SegmentChestTotalSegmentator), or (B) change the feature text to state it currently uses a single backend and keep the example; reference SegmentChestEnsemble, methods, and SegmentChestTotalSegmentator when making the change.docs/examples.rst (1)
106-109: Single-method ensemble may confuse users.The ensemble example now uses only
methods=['totalsegmentator'], which technically isn't an ensemble (ensembles combine multiple methods). While this aligns with the VISTA-3D removal, users may find it confusing to use an "ensemble" class with a single method.Consider either:
- Updating the example text to clarify that
SegmentChestEnsemblecurrently delegates to a single backend but maintains API stability- Recommending users call
SegmentChestTotalSegmentatordirectly for single-method segmentation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples.rst` around lines 106 - 109, Update the example to avoid confusion about using an "ensemble" with a single method: in the docs surrounding SegmentChestEnsemble mention that when methods=['totalsegmentator'] the class currently delegates to a single backend for API stability, and add a brief note recommending using SegmentChestTotalSegmentator directly for single-method cases; reference the SegmentChestEnsemble constructor (methods and fusion_strategy parameters) and the alternative SegmentChestTotalSegmentator symbol so readers can choose the clearer direct class for single-method segmentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/developer/segmentation.rst`:
- Around line 104-128: The docs example is inconsistent: SegmentChestEnsemble
claims to combine multiple methods but the example passes
methods=['totalsegmentator']; update the documentation to remove or clarify the
misleading parameter. Edit the example usage for SegmentChestEnsemble to either
(A) remove the methods argument and show direct instantiation (since
SegmentChestEnsemble delegates to SegmentChestTotalSegmentator), or (B) change
the feature text to state it currently uses a single backend and keep the
example; reference SegmentChestEnsemble, methods, and
SegmentChestTotalSegmentator when making the change.
In `@docs/examples.rst`:
- Around line 106-109: Update the example to avoid confusion about using an
"ensemble" with a single method: in the docs surrounding SegmentChestEnsemble
mention that when methods=['totalsegmentator'] the class currently delegates to
a single backend for API stability, and add a brief note recommending using
SegmentChestTotalSegmentator directly for single-method cases; reference the
SegmentChestEnsemble constructor (methods and fusion_strategy parameters) and
the alternative SegmentChestTotalSegmentator symbol so readers can choose the
clearer direct class for single-method segmentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62bbd42b-6451-4a24-a998-c4c46dfae4d4
📒 Files selected for processing (31)
.github/workflows/ci.ymlCLAUDE.mdREADME.mddocs/API_MAP.mddocs/README.mddocs/api/index.rstdocs/api/segmentation/ensemble.rstdocs/api/segmentation/index.rstdocs/api/segmentation/vista3d.rstdocs/api/segmentation/vista3d_nim.rstdocs/developer/architecture.rstdocs/developer/segmentation.rstdocs/examples.rstdocs/quickstart.rstdocs/testing.rstexperiments/Heart-GatedCT_To_USD/test_vista3d_class.pyexperiments/Heart-GatedCT_To_USD/test_vista3d_inMem.pyexperiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.pyexperiments/Lung-GatedCT_To_USD/Experiment_SegReg.pyexperiments/README.mdpyproject.tomlsrc/physiomotion4d/__init__.pysrc/physiomotion4d/cli/convert_ct_to_vtk.pysrc/physiomotion4d/segment_chest_ensemble.pysrc/physiomotion4d/workflow_convert_ct_to_vtk.pytests/GITHUB_WORKFLOWS.mdtests/README.mdtests/TESTING_GUIDE.mdtests/conftest.pytests/test_experiments.pytests/test_segment_chest_vista_3d.py
💤 Files with no reviewable changes (11)
- CLAUDE.md
- experiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.py
- tests/test_experiments.py
- docs/api/segmentation/vista3d_nim.rst
- docs/api/segmentation/vista3d.rst
- experiments/Lung-GatedCT_To_USD/Experiment_SegReg.py
- pyproject.toml
- experiments/Heart-GatedCT_To_USD/test_vista3d_class.py
- src/physiomotion4d/init.py
- tests/test_segment_chest_vista_3d.py
- experiments/Heart-GatedCT_To_USD/test_vista3d_inMem.py
✅ Files skipped from review due to trivial changes (4)
- src/physiomotion4d/cli/convert_ct_to_vtk.py
- experiments/README.md
- docs/README.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
Add return type and parameter annotations to every function in tests/, satisfying mypy disallow_untyped_defs=true. Extend mypy coverage from src/ only to src/ tests/ and add a pyproject.toml override so test modules ignore missing stubs from itk/pyvista. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… export Pass algorithm="dataset_surface" to every extract_surface() call to match the updated PyVista API. Switch segment_anatomy_base.py to the ITK snake_case functional API (resample_image_filter, etc.). Replace MultiplyImageFilter with explicit array multiply in transform_tools to avoid type-dispatch issues. Export TestTools from __init__ and add __test__=False to suppress spurious pytest collection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/physiomotion4d/segment_anatomy_base.py (1)
381-424:⚠️ Potential issue | 🔴 CriticalGuard empty connected-component results before selecting largest ID.
At Line 417,
np.argmax(component_sums)will raise when thresholding/masking yields no foreground component (idsempty). This can crash segmentation on valid edge inputs.💡 Suggested fix
ids = np.unique(connected_component_arr) ids = ids[ids != 0] + if ids.size == 0: + self.log_debug( + 'No connected components found for thresholds [%d, %d]; skipping update.', + lower_threshold, + upper_threshold, + ) + return labelmap_image component_sums = [np.sum(connected_component_arr == id) for id in ids] largest_id = ids[np.argmax(component_sums)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/physiomotion4d/segment_anatomy_base.py` around lines 381 - 424, The code computes ids = np.unique(connected_component_arr) then component_sums and uses largest_id = ids[np.argmax(component_sums)] without guarding for an empty ids array; add a check after ids = ids[ids != 0] to handle the empty case (e.g., if ids.size == 0) and short-circuit: either return an empty/zeroed connected_component_image (or a clear sentinel like None) or skip the binary_threshold_image_filter step and log/raise a descriptive error; ensure this guard is applied before calling np.argmax and before calling itk.binary_threshold_image_filter on connected_component_image so functions using connected_component_image (variables/functions: connected_component_arr, ids, component_sums, largest_id, connected_component_image, itk.binary_threshold_image_filter) won’t crash.tests/conftest.py (1)
414-425:⚠️ Potential issue | 🟠 MajorVersion or validate the cached ANTs transforms.
These cache filenames are reused unconditionally, but this fixture now registers
test_images[1]againsttest_images[7]. Any existingants_*_transform.hdffrom the previous frame pair will still be loaded locally, so the suite can silently use the wrong transform. Please include the frame indices in the cache key or persist metadata and verify it before reusing the files.Suggested direction
- inverse_transform_path = data_dir / "ants_inverse_transform.hdf" - forward_transform_path = data_dir / "ants_forward_transform.hdf" + frame_tag = "001_to_007" + inverse_transform_path = data_dir / f"ants_inverse_transform_{frame_tag}.hdf" + forward_transform_path = data_dir / f"ants_forward_transform_{frame_tag}.hdf"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/conftest.py` around lines 414 - 425, The cached transform files inverse_transform_path and forward_transform_path are reused unconditionally and may belong to different image pairs; update the fixture to version or validate the cache by incorporating the source-frame identifiers into the cache key (e.g., include the frame indices/ids from test_images used for registration when constructing the filenames) or by persisting/reading a small metadata file alongside the HDFs and comparing the stored source indices/hashes to the current test_images pair before calling itk.transformread; if the metadata or indices don't match, skip loading and re-run registration to generate new transforms.docs/examples.rst (1)
429-441:⚠️ Potential issue | 🟠 MajorUpdate these TotalSegmentator examples to the dict-based return API.
After switching to
SegmentChestTotalSegmentator, both snippets still treatsegment()as a positional tuple (masks[0]). The current API returns a result dict with alabelmap, so these examples now fail when copied verbatim.Suggested update
- masks = segmenter.segment(image, contrast_enhanced_study=True) - - # Save heart mask - output_name = filename.replace('.nrrd', '_heart.nrrd') - itk.imwrite(masks[0], output_name) + result = segmenter.segment(image, contrast_enhanced_study=True) + heart_mask = segmenter.create_anatomy_group_masks(result['labelmap'])['heart'] + + output_name = filename.replace('.nrrd', '_heart.nrrd') + itk.imwrite(heart_mask, output_name)- masks = segmenter.segment(reference, contrast_enhanced_study=True) - heart_mask = masks[0] + result = segmenter.segment(reference, contrast_enhanced_study=True) + heart_mask = segmenter.create_anatomy_group_masks(result['labelmap'])['heart']Also applies to: 521-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples.rst` around lines 429 - 441, The example treats the output of SegmentChestTotalSegmentator.segment() as a tuple (masks[0]) but the new API returns a dict with key "labelmap"; update the example to call segment(image, contrast_enhanced_study=True) and pull the resulting label map via result["labelmap"] (or assign e.g. result = segmenter.segment(...); labelmap = result["labelmap"]) before saving with itk.imwrite; apply the same change to the other snippet using SegmentChestTotalSegmentator.segment().tests/test_register_images_icon.py (1)
379-383:⚠️ Potential issue | 🟡 MinorFix misleading initial-offset log message.
Line 379 sets
[-5.0, -5.0, -5.0], but Line 382 prints[5.0, 5.0, 5.0]. This can confuse failure triage.Suggested diff
- print(" Initial offset: [5.0, 5.0, 5.0]") + print(" Initial offset: [-5.0, -5.0, -5.0]")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_register_images_icon.py` around lines 379 - 383, The log message is misleading: the test sets the initial offset via initial_tfm_forward.SetOffset([-5.0, -5.0, -5.0]) but prints "[5.0, 5.0, 5.0]"; update the print statement in tests/test_register_images_icon.py to reflect the actual offset (e.g., print " Initial offset: [-5.0, -5.0, -5.0]") so the printed message matches the value set by initial_tfm_forward.SetOffset.
🧹 Nitpick comments (2)
tests/test_contour_tools.py (1)
105-105: Narrowcontours_dicttype to concrete mesh type.Line 105 uses
dict[str, Any], but values appear to always bepv.PolyData. Usingdict[str, pv.PolyData]improves local type checking.Suggested diff
- contours_dict: dict[str, Any] = {} + contours_dict: dict[str, pv.PolyData] = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_contour_tools.py` at line 105, The local variable contours_dict is annotated too broadly as dict[str, Any]; narrow it to dict[str, pv.PolyData] to improve type checking by replacing the annotation on contours_dict with dict[str, pv.PolyData], and if pv is not already in scope ensure the pyvista alias (pv) is imported or referenced where the annotation is used (or use typing.TYPE_CHECKING + forward reference if necessary).tests/test_register_time_series_images.py (1)
165-168: Extract repeatedclass_nameliteral into a constant.
"registration_time_series_images"is repeated in multipleTestTools(...)calls; centralizing it reduces drift risk.Also applies to: 226-229, 400-403
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_register_time_series_images.py` around lines 165 - 168, Extract the repeated literal "registration_time_series_images" into a single constant (e.g., REG_CLASS_NAME or CLASS_NAME_REGISTRATION_TIME_SERIES_IMAGES) and replace all occurrences passed as the class_name parameter in TestTools(...) calls with that constant; update the file-level constants section (near other test constants) and ensure TestTools invocations at the other occurrences use the new constant so the same identifier is reused consistently across the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/faq.rst`:
- Around line 96-99: The FAQ lists only TotalSegmentator and Simpleware while
other docs still reference the "ensemble" method, causing a conflict; update the
FAQ entry to either include "ensemble" with its current status or explicitly
mark "ensemble" as removed/deprecated, and then make the same change across all
docs that mention "ensemble" (the troubleshooting, developer
segmentation/workflows, and CLI heart_gated_ct docs) so every place consistently
documents the supported segmentation methods (TotalSegmentator, Simpleware, and
ensemble if retained) and include a short note about deprecation/removal and
migration steps if you mark ensemble deprecated.
In `@src/physiomotion4d/convert_vtk_to_usd.py`:
- Line 404: Both mesh conversion paths call VTK's extract_surface but use
different arguments: update _split_by_labels to call
extract_surface(algorithm="dataset_surface") like _vtk_to_mesh_data so both
branches use the same extraction algorithm; locate the extract_surface call
inside the _split_by_labels function and add the algorithm="dataset_surface"
parameter to match the call in _vtk_to_mesh_data to ensure consistent mesh
outputs when mask_ids is enabled or disabled.
In `@src/physiomotion4d/workflow_fit_statistical_model_to_patient.py`:
- Around line 149-151: The extract_surface(...) calls pass a non-existent
algorithm keyword (algorithm="dataset_surface") which breaks on PyVista
<=0.42.0; remove the algorithm=... argument from all uses of extract_surface so
the method is invoked with only positional/default parameters (specifically
update the call that assigns self.template_model_surface via
self.template_model.extract_surface(...), the similar extract_surface calls in
the PCA branch, and the other two extract_surface usages around the
frame/template surface handling) to avoid TypeError at runtime.
In `@tests/conftest.py`:
- Around line 434-435: The hard-coded frame picks fixed_image = test_images[7]
and moving_image = test_images[1] can raise IndexError when the series is
shorter; update the fixture to guard these selections by verifying
len(test_images) >= 8 for the fixed frame and >= 2 for the moving frame and call
pytest.skip with a clear message if the checks fail (refer to the test_images
list and the fixed_image/moving_image assignments); alternatively, fall back to
valid indices (e.g., use the last frame for fixed_image and index 1 or 0 for
moving_image) only after the length checks to avoid silent IndexErrors.
In `@tests/test_convert_nrrd_4d_to_3d.py`:
- Around line 46-63: test_slice_files_created has a hidden dependency on
test_convert_4d_to_3d because it asserts files in output_dir without creating
them; make the test independent by either extracting the conversion call into a
shared fixture (e.g., create a fixture like convert_nrrd_4d_to_3d_fixture that
runs the conversion logic used in test_convert_4d_to_3d and returns the
output_dir) and use that fixture in test_slice_files_created, or explicitly
invoke the same conversion helper/function used by test_convert_4d_to_3d at the
start of test_slice_files_created to ensure slice files are generated before
doing the glob/asserts.
In `@tests/test_register_time_series_images.py`:
- Around line 173-182: The baseline-compare helpers
compare_result_to_baseline_transform and compare_result_to_baseline_image return
booleans instead of raising, so update each call site to assert their return
value (e.g., replace bare calls with assert
test_tools.compare_result_to_baseline_transform(...) and assert
test_tools.compare_result_to_baseline_image(...)) so the test fails on
mismatches; apply this change to all places where those helpers are invoked in
this test file (including the currently un-asserted calls surrounding
basic_time_series_registered_0.mha and basic_forward_transform_0.hdf).
---
Outside diff comments:
In `@docs/examples.rst`:
- Around line 429-441: The example treats the output of
SegmentChestTotalSegmentator.segment() as a tuple (masks[0]) but the new API
returns a dict with key "labelmap"; update the example to call segment(image,
contrast_enhanced_study=True) and pull the resulting label map via
result["labelmap"] (or assign e.g. result = segmenter.segment(...); labelmap =
result["labelmap"]) before saving with itk.imwrite; apply the same change to the
other snippet using SegmentChestTotalSegmentator.segment().
In `@src/physiomotion4d/segment_anatomy_base.py`:
- Around line 381-424: The code computes ids =
np.unique(connected_component_arr) then component_sums and uses largest_id =
ids[np.argmax(component_sums)] without guarding for an empty ids array; add a
check after ids = ids[ids != 0] to handle the empty case (e.g., if ids.size ==
0) and short-circuit: either return an empty/zeroed connected_component_image
(or a clear sentinel like None) or skip the binary_threshold_image_filter step
and log/raise a descriptive error; ensure this guard is applied before calling
np.argmax and before calling itk.binary_threshold_image_filter on
connected_component_image so functions using connected_component_image
(variables/functions: connected_component_arr, ids, component_sums, largest_id,
connected_component_image, itk.binary_threshold_image_filter) won’t crash.
In `@tests/conftest.py`:
- Around line 414-425: The cached transform files inverse_transform_path and
forward_transform_path are reused unconditionally and may belong to different
image pairs; update the fixture to version or validate the cache by
incorporating the source-frame identifiers into the cache key (e.g., include the
frame indices/ids from test_images used for registration when constructing the
filenames) or by persisting/reading a small metadata file alongside the HDFs and
comparing the stored source indices/hashes to the current test_images pair
before calling itk.transformread; if the metadata or indices don't match, skip
loading and re-run registration to generate new transforms.
In `@tests/test_register_images_icon.py`:
- Around line 379-383: The log message is misleading: the test sets the initial
offset via initial_tfm_forward.SetOffset([-5.0, -5.0, -5.0]) but prints "[5.0,
5.0, 5.0]"; update the print statement in tests/test_register_images_icon.py to
reflect the actual offset (e.g., print " Initial offset: [-5.0, -5.0, -5.0]")
so the printed message matches the value set by initial_tfm_forward.SetOffset.
---
Nitpick comments:
In `@tests/test_contour_tools.py`:
- Line 105: The local variable contours_dict is annotated too broadly as
dict[str, Any]; narrow it to dict[str, pv.PolyData] to improve type checking by
replacing the annotation on contours_dict with dict[str, pv.PolyData], and if pv
is not already in scope ensure the pyvista alias (pv) is imported or referenced
where the annotation is used (or use typing.TYPE_CHECKING + forward reference if
necessary).
In `@tests/test_register_time_series_images.py`:
- Around line 165-168: Extract the repeated literal
"registration_time_series_images" into a single constant (e.g., REG_CLASS_NAME
or CLASS_NAME_REGISTRATION_TIME_SERIES_IMAGES) and replace all occurrences
passed as the class_name parameter in TestTools(...) calls with that constant;
update the file-level constants section (near other test constants) and ensure
TestTools invocations at the other occurrences use the new constant so the same
identifier is reused consistently across the file.
🪄 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: 663c2d5d-b591-4166-9421-69082c803f76
📒 Files selected for processing (58)
CLAUDE.mdREADME.mddata/test/.gitignoredocs/API_MAP.mddocs/api/index.rstdocs/api/segmentation/base.rstdocs/api/segmentation/ensemble.rstdocs/api/segmentation/index.rstdocs/api/segmentation/totalsegmentator.rstdocs/api/workflows.rstdocs/architecture.rstdocs/developer/architecture.rstdocs/developer/segmentation.rstdocs/examples.rstdocs/faq.rstdocs/index.rstdocs/installation.rstdocs/quickstart.rstdocs/references.rstexperiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.pyexperiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.pyexperiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.pypyproject.tomlsrc/physiomotion4d/__init__.pysrc/physiomotion4d/contour_tools.pysrc/physiomotion4d/convert_vtk_to_usd.pysrc/physiomotion4d/register_models_distance_maps.pysrc/physiomotion4d/register_models_icp.pysrc/physiomotion4d/register_models_icp_itk.pysrc/physiomotion4d/segment_anatomy_base.pysrc/physiomotion4d/segment_chest_ensemble.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/transform_tools.pysrc/physiomotion4d/workflow_create_statistical_model.pysrc/physiomotion4d/workflow_fit_statistical_model_to_patient.pytests/TESTING_GUIDE.mdtests/baselines/TestRegisterTimeSeriesImages/basic_time_series_registered_0.mhatests/baselines/TestRegisterTimeSeriesImages/prior_time_series_registered_0.mhatests/baselines/TestRegisterTimeSeriesImages/transform_application_time_series_0.mhatests/conftest.pytests/test_contour_tools.pytests/test_convert_nrrd_4d_to_3d.pytests/test_convert_vtk_to_usd.pytests/test_download_heart_data.pytests/test_experiments.pytests/test_image_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_transform_tools.pytests/test_usd_merge.pytests/test_usd_time_preservation.pytests/test_vtk_to_usd_library.py
💤 Files with no reviewable changes (3)
- docs/references.rst
- src/physiomotion4d/segment_chest_ensemble.py
- docs/api/segmentation/ensemble.rst
✅ Files skipped from review due to trivial changes (21)
- data/test/.gitignore
- docs/installation.rst
- docs/api/workflows.rst
- docs/architecture.rst
- tests/baselines/TestRegisterTimeSeriesImages/basic_time_series_registered_0.mha
- tests/baselines/TestRegisterTimeSeriesImages/prior_time_series_registered_0.mha
- src/physiomotion4d/test_tools.py
- src/physiomotion4d/register_models_icp.py
- src/physiomotion4d/register_models_distance_maps.py
- src/physiomotion4d/register_models_icp_itk.py
- tests/baselines/TestRegisterTimeSeriesImages/transform_application_time_series_0.mha
- docs/index.rst
- docs/api/segmentation/base.rst
- docs/quickstart.rst
- docs/developer/segmentation.rst
- docs/developer/architecture.rst
- tests/TESTING_GUIDE.md
- README.md
- tests/test_usd_merge.py
- docs/api/segmentation/index.rst
- docs/api/segmentation/totalsegmentator.rst
🚧 Files skipped from review as they are similar to previous changes (4)
- pyproject.toml
- docs/api/index.rst
- CLAUDE.md
- tests/test_experiments.py
Comment on lines
+149
to
+151
| self.template_model_surface = template_model.extract_surface( | ||
| algorithm="dataset_surface" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PyVista 0.42.0, what is the exact signature of DataSetFilters.extract_surface, and does it support an algorithm keyword argument?
💡 Result:
In PyVista 0.42.0, the exact signature of DataSetFilters.extract_surface is: DataSetFilters.extract_surface(pass_pointid=True, pass_cellid=True, nonlinear_subdivision=1, progress_bar=False) It does not support an algorithm keyword argument. The algorithm parameter was added in version 0.47.0.
Citations:
- 1: https://pyvista-0-42.netlify.app/api/core/_autosummary/pyvista.datasetfilters.extract_surface
- 2: https://docs.pyvista.org/api/core/_autosummary/pyvista.DataObjectFilters.extract_surface.html
- 3: https://pyvista-0-42.netlify.app/api/core/_autosummary/pyvista.datasetfilters
- 4: https://pyvista-0-42.netlify.app/examples/01-filter/extract-surface
extract_surface(algorithm='dataset_surface') is incompatible with PyVista 0.42.0 and will cause runtime failure.
Lines 149–151, 171–178, and 574–576 use the algorithm keyword argument, which does not exist in PyVista 0.42.0. The parameter was added only in version 0.47.0. These calls will raise TypeError at runtime during workflow initialization and the PCA branch.
Remove the algorithm parameter from all four extract_surface() calls:
Required changes
- self.template_model_surface = template_model.extract_surface(
- algorithm="dataset_surface"
- )
+ self.template_model_surface = template_model.extract_surface()
- patient_models_surfaces = [
- model.extract_surface(algorithm="dataset_surface")
- for model in patient_models
- ]
+ patient_models_surfaces = [model.extract_surface() for model in patient_models]
- self.patient_model_surface = self.combined_patient_model.extract_surface(
- algorithm="dataset_surface"
- )
+ self.patient_model_surface = self.combined_patient_model.extract_surface()
- self.pca_template_model_surface = result[
- "registered_model"
- ].extract_surface(algorithm="dataset_surface")
+ self.pca_template_model_surface = result["registered_model"].extract_surface()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/physiomotion4d/workflow_fit_statistical_model_to_patient.py` around lines
149 - 151, The extract_surface(...) calls pass a non-existent algorithm keyword
(algorithm="dataset_surface") which breaks on PyVista <=0.42.0; remove the
algorithm=... argument from all uses of extract_surface so the method is invoked
with only positional/default parameters (specifically update the call that
assigns self.template_model_surface via
self.template_model.extract_surface(...), the similar extract_surface calls in
the PCA branch, and the other two extract_surface usages around the
frame/template surface handling) to avoid TypeError at runtime.
Comment on lines
+434
to
435
| fixed_image = test_images[7] | ||
| moving_image = test_images[1] |
There was a problem hiding this comment.
Guard the hard-coded frame selection.
test_images only skips when fewer than 3 time points are present, but this fixture immediately indexes [7]. If conversion/download leaves only a partial series behind, this fails with IndexError instead of producing a clear skip/fail reason.
Suggested fix
# Perform registration if files don't exist or loading failed
print("\nPerforming ANTs registration...")
+ if len(test_images) <= 7:
+ pytest.skip(
+ f"Need at least 8 resampled time points for ANTs registration, found {len(test_images)}"
+ )
fixed_image = test_images[7]
moving_image = test_images[1]📝 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.
Suggested change
| fixed_image = test_images[7] | |
| moving_image = test_images[1] | |
| # Perform registration if files don't exist or loading failed | |
| print("\nPerforming ANTs registration...") | |
| if len(test_images) <= 7: | |
| pytest.skip( | |
| f"Need at least 8 resampled time points for ANTs registration, found {len(test_images)}" | |
| ) | |
| fixed_image = test_images[7] | |
| moving_image = test_images[1] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/conftest.py` around lines 434 - 435, The hard-coded frame picks
fixed_image = test_images[7] and moving_image = test_images[1] can raise
IndexError when the series is shorter; update the fixture to guard these
selections by verifying len(test_images) >= 8 for the fixed frame and >= 2 for
the moving frame and call pytest.skip with a clear message if the checks fail
(refer to the test_images list and the fixed_image/moving_image assignments);
alternatively, fall back to valid indices (e.g., use the last frame for
fixed_image and index 1 or 0 for moving_image) only after the length checks to
avoid silent IndexErrors.
Comment on lines
+46
to
63
| 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.""" | ||
| data_dir = test_directories["data"] | ||
| output_dir = test_directories["output"] / "convert_nrrd_4d_to_3d" | ||
| output_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Check that slice files exist | ||
| slice_files = list(data_dir.glob("slice_*.mha")) | ||
| slice_files = list(output_dir.glob("slice_*.mha")) | ||
| assert len(slice_files) > 10, ( | ||
| f"Expected more than 10 slice files, found {len(slice_files)}" | ||
| ) | ||
|
|
||
| # Verify specific slice file exists (mid-stroke) | ||
| slice_007 = data_dir / "slice_007.mha" | ||
| slice_007 = output_dir / "slice_007.mha" | ||
| assert slice_007.exists(), "Expected slice_007.mha not found" |
There was a problem hiding this comment.
Remove the cross-test ordering dependency.
test_slice_files_created() now only checks output_dir, but nothing in this test creates those files. If pytest runs it before test_convert_4d_to_3d(), the directory is empty and this fails spuriously. Please move the conversion into a shared fixture or rerun it here before asserting.
Suggested 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_nrrd_4d_to_3d"
output_dir.mkdir(parents=True, exist_ok=True)
+
+ if not (output_dir / "slice_007.mha").exists():
+ conv = ConvertNRRD4DTo3D()
+ conv.load_nrrd_4d(str(download_test_data))
+ conv.save_3d_images(str(output_dir / "slice"))
# Check that slice files exist
slice_files = list(output_dir.glob("slice_*.mha"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_convert_nrrd_4d_to_3d.py` around lines 46 - 63,
test_slice_files_created has a hidden dependency on test_convert_4d_to_3d
because it asserts files in output_dir without creating them; make the test
independent by either extracting the conversion call into a shared fixture
(e.g., create a fixture like convert_nrrd_4d_to_3d_fixture that runs the
conversion logic used in test_convert_4d_to_3d and returns the output_dir) and
use that fixture in test_slice_files_created, or explicitly invoke the same
conversion helper/function used by test_convert_4d_to_3d at the start of
test_slice_files_created to ensure slice files are generated before doing the
glob/asserts.
np.argmax on an empty array raises ValueError; this can occur when the input contains no labeled voxels in the threshold range (e.g. a non-contrast study).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/API_MAP.md`:
- Line 562: Update the source test docstrings that generate docs/API_MAP.md:
find the docstring for the test function named test_imwrite_imread_vd3 and
change “double precision”/hyphenation to “double-precision” (and fix the
spelling/orthography in the other test docstring referenced around the content
that generates line 663), then re-run the documentation generator to regenerate
docs/API_MAP.md so the corrected phrases replace the broken hyphenation and
spelling in the generated file.
In `@src/physiomotion4d/segment_anatomy_base.py`:
- Around line 418-419: The log message inside segment_connected_component uses
double-quoted string literals; change them to single-quoted non-docstring
strings (e.g., replace the double-quoted concatenated message
"segment_connected_component: no connected components found " "in threshold [%d,
%d]; returning labelmap unchanged" with an equivalent single-quoted literal) so
it follows the repo style for Python string quoting used in functions/methods in
segment_anatomy_base.py.
🪄 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: 7ae97fd0-68cb-4fb8-babb-7b002fd37ea2
📒 Files selected for processing (25)
docs/API_MAP.mddocs/README.mddocs/api/registration/index.rstdocs/api/segmentation/totalsegmentator.rstdocs/cli_scripts/heart_gated_ct.rstdocs/developer/workflows.rstdocs/examples.rstdocs/troubleshooting.rstpyproject.tomlsrc/physiomotion4d/convert_vtk_to_usd.pysrc/physiomotion4d/segment_anatomy_base.pysrc/physiomotion4d/test_tools.pytests/baselines/TestRegisterTimeSeriesImages/basic_forward_transform_0.hdftests/baselines/TestRegisterTimeSeriesImages/basic_time_series_registered_0.mhatests/baselines/TestRegisterTimeSeriesImages/prior_forward_transform_0.hdftests/baselines/TestRegisterTimeSeriesImages/prior_time_series_registered_0.mhatests/baselines/registration_time_series_images/basic_forward_transform_0.hdftests/baselines/registration_time_series_images/basic_time_series_registered_0.mhatests/baselines/registration_time_series_images/prior_forward_transform_0.hdftests/baselines/registration_time_series_images/prior_time_series_registered_0.mhatests/baselines/registration_time_series_images/transform_application_time_series_0.mhatests/conftest.pytests/test_contour_tools.pytests/test_register_images_icon.pytests/test_register_time_series_images.py
💤 Files with no reviewable changes (4)
- tests/baselines/TestRegisterTimeSeriesImages/prior_forward_transform_0.hdf
- tests/baselines/TestRegisterTimeSeriesImages/basic_forward_transform_0.hdf
- tests/baselines/TestRegisterTimeSeriesImages/prior_time_series_registered_0.mha
- tests/baselines/TestRegisterTimeSeriesImages/basic_time_series_registered_0.mha
✅ Files skipped from review due to trivial changes (12)
- tests/baselines/registration_time_series_images/basic_time_series_registered_0.mha
- docs/api/registration/index.rst
- docs/cli_scripts/heart_gated_ct.rst
- docs/developer/workflows.rst
- docs/troubleshooting.rst
- tests/baselines/registration_time_series_images/basic_forward_transform_0.hdf
- tests/baselines/registration_time_series_images/prior_time_series_registered_0.mha
- src/physiomotion4d/convert_vtk_to_usd.py
- tests/baselines/registration_time_series_images/transform_application_time_series_0.mha
- tests/baselines/registration_time_series_images/prior_forward_transform_0.hdf
- docs/README.md
- docs/examples.rst
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/api/segmentation/totalsegmentator.rst
- pyproject.toml
- src/physiomotion4d/test_tools.py
- tests/conftest.py
- tests/test_contour_tools.py
- tests/test_register_time_series_images.py
- tests/test_register_images_icon.py
| - `def test_itk_to_sitk_vector_image(self, image_tools)` (line 133): Test conversion of vector ITK image to SimpleITK. | ||
| - `def test_sitk_to_itk_vector_image(self, image_tools)` (line 171): Test conversion of vector SimpleITK image to ITK. | ||
| - `def test_roundtrip_vector_image(self, image_tools)` (line 202): Test roundtrip conversion for vector images: ITK -> SimpleITK -> ITK. | ||
| - `def test_imwrite_imread_vd3(self, image_tools, test_transforms, test_images, test_directories)` (line 241): Test reading and writing double precision vector images. |
There was a problem hiding this comment.
Fix doc text issues in source docstrings, then regenerate this file.
Line 562 has a hyphenation issue (“double-precision”), and Line 663 has a spelling/orthography warning. Since this file is generated, fix the corresponding source docstrings in test modules and re-run the generator.
Also applies to: 663-663
🧰 Tools
🪛 LanguageTool
[grammar] ~562-~562: Use a hyphen to join words.
Context: ...ne 241): Test reading and writing double precision vector images. - **class TestF...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/API_MAP.md` at line 562, Update the source test docstrings that generate
docs/API_MAP.md: find the docstring for the test function named
test_imwrite_imread_vd3 and change “double precision”/hyphenation to
“double-precision” (and fix the spelling/orthography in the other test docstring
referenced around the content that generates line 663), then re-run the
documentation generator to regenerate docs/API_MAP.md so the corrected phrases
replace the broken hyphenation and spelling in the generated file.
Comment on lines
+418
to
+419
| "segment_connected_component: no connected components found " | ||
| "in threshold [%d, %d]; returning labelmap unchanged", |
There was a problem hiding this comment.
Use single quotes for non-docstring string literals.
Line 418–419 introduces a double-quoted log string; repository style requires single quotes for standard strings.
Style-only fix
- "segment_connected_component: no connected components found "
- "in threshold [%d, %d]; returning labelmap unchanged",
+ 'segment_connected_component: no connected components found '
+ 'in threshold [%d, %d]; returning labelmap unchanged',📝 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.
Suggested change
| "segment_connected_component: no connected components found " | |
| "in threshold [%d, %d]; returning labelmap unchanged", | |
| 'segment_connected_component: no connected components found ' | |
| 'in threshold [%d, %d]; returning labelmap unchanged', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/physiomotion4d/segment_anatomy_base.py` around lines 418 - 419, The log
message inside segment_connected_component uses double-quoted string literals;
change them to single-quoted non-docstring strings (e.g., replace the
double-quoted concatenated message "segment_connected_component: no connected
components found " "in threshold [%d, %d]; returning labelmap unchanged" with an
equivalent single-quoted literal) so it follows the repo style for Python string
quoting used in functions/methods in segment_anatomy_base.py.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 88 out of 88 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
283
to
+292
| # Try to download if not found locally | ||
| input_image_url = "https://github.com/Slicer-Heart-CT/Slicer-Heart-CT/releases/download/TestingData/TruncalValve_4DCT.seq.nrrd" | ||
| input_image_url = "https://github.com/SlicerHeart/SlicerHeart/releases/download/TestingData/TruncalValve_4DCT.seq.nrrd" | ||
| print(f"\nDownloading TruncalValve 4D CT data from {input_image_url}...") | ||
|
|
||
| try: | ||
| urllib.request.urlretrieve(input_image_url, str(input_image_filename)) | ||
| print(f"Downloaded to {input_image_filename}") | ||
| except urllib.error.HTTPError as e: | ||
| pytest.skip( | ||
| f"Could not download test data: {e}. Please manually place TruncalValve_4DCT.seq.nrrd in {data_dir}" | ||
| except urllib.error.URLError as e: | ||
| msg = ( | ||
| f"Could not download test data: {e}. " |
| .. code-block:: python | ||
|
|
||
| processor.set_segmentation_method('ensemble') | ||
| processor.set_segmentation_method('simpleware') |
|
|
||
| - **Complete 4D Medical Imaging Pipeline**: End-to-end processing from 4D CT data to animated USD models | ||
| - **Multiple AI Segmentation Methods**: TotalSegmentator, VISTA-3D, and ensemble approaches | ||
| - **Multiple AI Segmentation Methods**: TotalSegmentator and ensemble approaches |
| * **TotalSegmentator**: Whole-body CT segmentation (100+ structures) | ||
| * **VISTA-3D**: MONAI-based foundation model for medical imaging | ||
| * **VISTA-3D NIM**: NVIDIA Inference Microservice version | ||
| * **Ensemble**: Combine multiple methods for improved accuracy |
Comment on lines
+46
to
59
| 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.""" | ||
| data_dir = test_directories["data"] | ||
| output_dir = test_directories["output"] / "convert_nrrd_4d_to_3d" | ||
| output_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Check that slice files exist | ||
| slice_files = list(data_dir.glob("slice_*.mha")) | ||
| slice_files = list(output_dir.glob("slice_*.mha")) | ||
| assert len(slice_files) > 10, ( | ||
| f"Expected more than 10 slice files, found {len(slice_files)}" | ||
| ) |
…nces
urllib.error was used in conftest.py but not imported, causing AttributeError on download failures. Also corrects the segmentation method identifier in troubleshooting docs ('simpleware' -> 'simpleware_heart') and removes stale ensemble references from README and developer docs.
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.
Summary by CodeRabbit
Documentation
Chores
Refactor
Tests