Refactor#62
Open
Hendrik-code wants to merge 21 commits into
Open
Conversation
Extract cutout size, sacrum ids, cleaning thresholds, spatial-dims count and the default label mapping in Segmentation_Inference_Config into named module-level constants. Simplify the citation_reminder wrapper's return. Behavior-preserving (modelconfig/enums tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ath code - phase_post: extract IVD/endplate label offsets (100/200), C2 instance label, crop margin, dilation cap, merge/dominance ratios and contact threshold. - find_min_cost_path: name the special vertebra class indices (T11/T12/L5), region start indices, softmax temperature and per-class repeat limit; copy region starts so the later append() no longer risks mutating a shared list. - proc_functions: name the vertebra instance-label upper bound. Pure literal->named-constant substitutions; full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- seg_pipeline: name IVD/endplate label ranges and fold the two label-strip loops into one map; use v_name2idx["S1"] instead of the bare 26 for the sacrum. - seg_utils: drop unused len_ignored_text dead variable. - phase_pre: name the [0, 1500] normalization range and the Vibe crop margin. - phase_semantic: name the small-CC threshold, canal height margin, sacrum label, expected-CC count and bounding-box crop margin. Behavior-preserving; full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- get_models: collapse the three near-identical get_*_model functions into a shared _get_model_by_name helper; unify the 'no available models' message (also fixes the 'modelweights' typo). Tests only assert the raised KeyError, not the message text, so behavior is preserved. - seg_model: name the zoom-match tolerance (1e-4) and the legacy Unet3D label normalization factor (9). Full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- phase_labeling: replace literal 24/6 cost-matrix widths with VERT_CLASSES and len(VertRel) (resolves the author's '# TODO 24/6 fix?'); name the special vertebra class indices (C1/C2/T11/T12/L5), region starts, T13 post-proc label and the 128mm labeling crop margin. - lab_model: name the default classifier input patch size (152, 168, 32). Verified len(VertExact)==24, len(VertRel)==6, T12==18, L5==23 before substituting. Full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- phase_instance: name the crop margin (5), minimum voxel threshold (40), IVD connected-component label offset (100), merged-corpus heuristics (same-height threshold, neighbor window, min neighbors, volume ratio); use Location.Vertebra_Corpus_border.value instead of the bare 49. - seg_run: use Location.Vertebra_Corpus(_border).value for the 50->49 remap. Behavior-preserving; full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add module-, class-, function- and method-level Google-style docstrings to all non-vendored modules (core pipeline, phases, models, utils, architectures, architectures_new, examples). Replace the auto-generated _type_/_description_ placeholder docstrings and convert the few NumPy/rST-style docstrings to Google style. Documentation-only change: no code logic was modified (verified by diff review and the full test suite). Also removed two now-redundant 'pass' statements in abstract methods and kept example scripts' INP001 suppression intact. Vendored/adapted files (nnUNet utils, image.py from SCT, inference_api.py) were left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… raising 'name in SomeEnum' did 'cls[item]' and only caught ValueError, but member-name lookup on a missing name raises KeyError (Python 3.11), so membership tests on non-members raised instead of returning False (the documented behavior). Catch KeyError as well. Surfaced while adding unit tests for the enums. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add behavior-focused unit tests for pure (no GPU / no model weights) functions: - test_phase_labeling.py (39): region_to_vert, prepare_vert/vertgrp/region/vertrel, prepare_vertrel_columns/vertt13_columns/visible, fpath_post_processing, is_valid_vertebra_sequence. - test_find_min_cost_path.py (32): argmin, softmax_T, c_to_region_idx, internal_to_real_path and many find_most_probably_sequence scenarios. - test_generate_disc_labels_extra.py (18): project_point_on_line, extract_centroids_3d, closest_point_seg_to_line, default_name_discs, DISCS_MAP. - test_pure_helpers.py (20): Modality/Acquisition.format_keys, Enum_Compare/MetaEnum, overlap_slice, find_nearest_lower, add_ignore_text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a MkDocs + Material + mkdocstrings documentation site, mirroring the TPTBox repository's setup: - mkdocs.yml: Material theme, mkdocstrings (Google docstrings), matching plugins, markdown extensions and a Home/Getting Started/Modules/API Reference nav. - .readthedocs.yml: ReadTheDocs build config (ubuntu-24.04, py3.12). - docs/: index.md, getting-started.md, narrative module pages (pipeline, phases, models) and auto-generated API reference pages (pipeline, models, phases, enums, utils, architectures) via mkdocstrings ':::' directives. - pyproject.toml: optional 'docs' dependency group (mkdocs, mkdocs-material, mkdocstrings[python]). - README: add a Documentation section. Verified with 'mkdocs build' (site builds successfully; remaining output is non-fatal griffe annotation warnings). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add parameter and return-type annotations across non-vendored modules so the generated API docs render full signatures. Reduces 'No type or annotation' warnings from 115 to 6 (the remainder are idiomatic pass-through **kwargs). Signature-only changes; under 'from __future__ import annotations' these are pure type hints with no runtime effect (one TYPE_CHECKING-guarded import added in phase_pre). Restored a displaced '# noqa: ARG002' on Unet3D.forward's unused conditioning args. Full test suite passes and ruff is clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… 52%) Emulate the networks with unittest.mock (reusing the test_semantic.py dummy-model pattern) so the inference orchestration is exercised without weights or a GPU: - predict_instance_mask: mock segment_scan to return a 1/2/3 three-vertebra cutout split, driving the full cutout-collection + Dice-merge pipeline; plus the no-corpus EMPTY path. (phase_instance 5% -> 60%) - perform_labeling_step / run_model_for_vert_labeling: mock run_all_seg_instances with a deterministic VERT head. (phase_labeling 18% -> 85%) - Segmentation_Model.segment_scan: padding round-trip and output mapping with a mocked run(). 148 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a FakeClassifierPredictor (eval/to/forward/softmax returning deterministic multi-head logits) and exercise VertLabelingClassifier's real inference path on CPU, mocking only the network: - _run_array (with and without an explicit seg): MONAI ToTensor + transform, predictor.forward/softmax, argmax. - run_all_arrays: per-array classification. - run_all_seg_instances: full reorient -> per-instance cutout (run_given_seg_pos / run_given_center_pos) -> _run_array path over every label in the mask. lab_model.py 27% -> 77%; overall coverage 52% -> 54%. 152 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hendrik-code
added
documentation
Add spineps/utils/resolution.py with a REFERENCE_ZOOM (0.75x0.75x1.65, the T2w model resolution) and helpers to convert physical mm/mm2/mm3 thresholds to voxels at the actual image zoom. Convert the instance-phase voxel thresholds to physical units: crop margin, the corpus/vertebra cleaning floor (mm3), and the same-height merge threshold (along the inferior axis). At the reference resolution the converted voxel counts equal the originals, so T2w results are unchanged; other resolutions (e.g. CT 0.8mm iso) now scale physically. Full test suite (152) passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…port, step 2) Convert the remaining internal voxel thresholds to physical units, converted to voxels at the actual image zoom: - phase_semantic: small-CC cleaning volume (mm3) and bounding-box crop margin (mm). - phase_post: crop margin (mm) and the merged-vertebra contact threshold, now an orientation-agnostic area (mm2 via geometric-mean voxel area). - phase_pre: Vibe spine-crop margin (mm). - resolution.py: replace the unused in-plane area helper with isotropic_area_to_voxels. User-facing parameters (pad_size, proc_pad_size, auto_crop_* ) are left as caller knobs. Behavior is unchanged at the 0.75x0.75x1.65 reference; full suite (152) passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ocstrings - test_resolution.py (8 tests, resolution.py at 100%): verify mm<->voxel helpers reproduce the original voxel counts at the reference resolution and scale physically at finer (CT 0.8mm) and coarser (1.5mm) spacings. - Update docstrings that still named the old voxel constants to the new *_MM names. 160 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each shared constant now has a single canonical home and is imported elsewhere (following the existing import direction, so no cycles): - T11_CLASS_IDX / T12_CLASS_IDX / L5_CLASS_IDX and the region starts: keep in find_min_cost_path (the path solver that consumes them); phase_labeling imports them and drops its copies (REGION_STARTS -> DEFAULT_REGION_STARTS). - IVD_LABEL_OFFSET / ENDPLATE_LABEL_OFFSET: define once in seg_pipeline (lowest level; its label ranges now derive from them); phase_post imports them, and phase_instance reuses IVD_LABEL_OFFSET in place of its duplicate IVD_CC_LABEL_OFFSET. Coincidentally-equal but semantically distinct values (the three 0.5 ratios, C2_INSTANCE_LABEL=2 vs C2_CLASS_IDX=1) were intentionally left separate. Values verified unchanged; 160 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a broad refactor/documentation push that also introduces resolution-aware threshold conversion utilities and a large new unit-test suite to lock down helper behavior (resolution conversions, labeling/path solver, disc labeling, mocked inference).
Changes:
- Added extensive
unittestcoverage for core helper functions and several inference/labeling code paths (mocked models). - Introduced resolution-aware mm/mm²/mm³ → voxel conversion helpers and wired semantic post-processing thresholds to scale by input zoom.
- Added/expanded docstrings across many modules and introduced MkDocs + ReadTheDocs configuration and initial docs pages.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| unit_tests/test_resolution.py | New tests for resolution-aware mm/mm²/mm³-to-voxel helpers. |
| unit_tests/test_pure_helpers.py | New tests for enums, overlap_slice, find_nearest_lower, and add_ignore_text. |
| unit_tests/test_phase_labeling.py | New tests for labeling pre/post-processing helper functions. |
| unit_tests/test_inference_mocked.py | Mocked tests exercising orchestration around segmentation + labeling models without GPU/weights. |
| unit_tests/test_generate_disc_labels_extra.py | New tests for disc-label generation geometry helpers and mappings. |
| unit_tests/test_find_min_cost_path.py | New tests for min-cost path utilities and constraints. |
| spineps/utils/seg_modelconfig.py | Introduces constants/defaults and improves typing/docstrings for inference config parsing. |
| spineps/utils/resolution.py | Adds reference zoom constants and mm/mm²/mm³→voxel conversion helpers. |
| spineps/utils/proc_functions.py | Adds constants and improves typing/docstrings; small logic tweak for instance label range constant. |
| spineps/utils/generate_disc_labels.py | Adds typing/docstrings to CLI + helpers. |
| spineps/utils/find_min_cost_path.py | Adds constants, typing/docstrings; replaces magic numbers with named constants. |
| spineps/utils/filepaths.py | Adds module docstring. |
| spineps/utils/compat.py | Adds typing to zip_strict and imports Iterable. |
| spineps/utils/citation_reminder.py | Simplifies wrapper return; adds docstring. |
| spineps/utils/auto_download.py | Adds typing/docstrings to download helpers. |
| spineps/utils/init.py | Adds package docstring. |
| spineps/seg_utils.py | Adds typing/docstrings for compatibility checks and ignore-text helper. |
| spineps/seg_run.py | Large docstring/type-annotation pass; replaces hard-coded corpus relabel with Location enum usage. |
| spineps/seg_pipeline.py | Adds label-offset constants and docstrings; simplifies derived-label stripping map. |
| spineps/seg_model.py | Adds constants/docstrings; tweaks output space mapping in segment_scan; replaces magic numbers. |
| spineps/seg_enums.py | Adds docstrings/typing; fixes MetaEnum contains to catch KeyError. |
| spineps/phase_semantic.py | Adds resolution-aware thresholds and docstrings; refines bounding-box cleaning margins. |
| spineps/phase_pre.py | Adds constants/docstrings; makes VIBESeg crop margin resolution-aware. |
| spineps/lab_model.py | Adds constants/docstrings/typing; exposes default classifier input size constant. |
| spineps/get_models.py | Refactors model lookup into shared helper; improves error message reuse and typing/docstrings. |
| spineps/example/template_roll_out.py | Adds script/module docstrings. |
| spineps/example/helper_parallel.py | Adds script/module docstrings. |
| spineps/example/get_gpu.py | Adds docstrings to GPU helpers. |
| spineps/entrypoint.py | Adds docstrings to CLI wiring functions. |
| spineps/architectures/unet3D.py | Adds docstrings and typing improvements to forward path and blocks. |
| spineps/architectures/read_labels.py | Adds docstrings/typing improvements for label mappings and targets. |
| spineps/architectures/pl_unet.py | Adds docstrings/typing to Lightning module wrapper. |
| spineps/architectures/pl_densenet.py | Adds docstrings/typing to classifier backbones and heads. |
| spineps/architectures/init.py | Adds module docstring. |
| spineps/architectures_new/unet3D.py | Adds docstrings/typing to new 3D U-Net implementation. |
| spineps/architectures_new/unet2D.py | Adds docstrings/typing to new 2D U-Net implementation. |
| spineps/architectures_new/pl_unet.py | Adds docstrings/typing to new Lightning wrapper. |
| spineps/architectures_new/dice.py | Adds docstrings/typing to Dice loss. |
| spineps/architectures_new/init.py | Adds module docstring. |
| spineps/init.py | Adds package docstring. |
| README.md | Adds documentation section and local MkDocs build instructions. |
| pyproject.toml | Adds optional docs dependency group for MkDocs tooling. |
| mkdocs.yml | Adds MkDocs (Material) configuration and nav structure. |
| docs/modules/pipeline.md | New docs page describing pipeline structure and outputs. |
| docs/modules/phases.md | New docs page describing processing phases and labels. |
| docs/modules/models.md | New docs page describing model loading and VERIDAH labeling. |
| docs/index.md | New docs landing page with overview and quick start. |
| docs/getting-started.md | New getting-started guide (install, weights, CLI/Python usage). |
| docs/api/utils.md | New API reference page for utils modules (mkdocstrings). |
| docs/api/pipeline.md | New API reference page for pipeline/run modules. |
| docs/api/phases.md | New API reference page for phase modules. |
| docs/api/models.md | New API reference page for models/loading modules. |
| docs/api/enums.md | New API reference page for enums + inference config. |
| docs/api/architectures.md | New API reference page for architectures/label definitions. |
| .readthedocs.yml | Adds ReadTheDocs config for building MkDocs-based documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
138
to
141
| self_zms = self.calc_recommended_resampling_zoom(input_zoom=input_zoom) | ||
| model_zms = model.calc_recommended_resampling_zoom(input_zoom=self_zms) | ||
| match: bool = bool(np.all([self_zms[i] - model_zms[i] < 1e-4 for i in range(3)])) | ||
| match: bool = bool(np.all([self_zms[i] - model_zms[i] < ZOOM_MATCH_TOLERANCE for i in range(3)])) | ||
| return match |
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.
No description provided.