Add rtmask-conformance gate as opt-in CI check - #55
Open
brianmanderson wants to merge 9 commits into
Open
Conversation
Wire the analytic-ground-truth conformance suite (rtmask-conformance) in
as an opt-in test gate, mirroring the four-piece pattern used in
DicomRTTool and PyRaDiSe:
* setup.py — new `conformance` extra; default install unaffected.
* dcmrtstruct2nii/tests/test_conformance.py — pytest fixtures generate
the synthetic CT+RTSTRUCT, drive dcmrtstruct2nii(), rename
`mask_<roi>.nii.gz` to `<roi>.nii.gz`, and assert per-ROI via
rtmask_conformance.evaluate_one. Skips cleanly when the extra isn't
installed (importorskip).
* dcmrtstruct2nii/tests/conformance.yaml — empty schema-versioned
placeholder for future threshold relaxations.
* .github/workflows/conformance.yml — standalone Conformance CI on
ubuntu-latest, Python 3.12, with workflow_dispatch.
Also fix two latent pydicom 3.x incompatibilities the gate exposes
(pydicom.read_file was removed in 3.0). One-line rename to
pydicom.dcmread; behavior is unchanged. The existing test suite
benefits from this fix too.
Local run: 7/7 ROIs PASS on the published default thresholds — no
per-ROI relaxation required, including cube (suggesting scikit-image's
polygon() rasterization doesn't carry the cv2-fillPoly +3.36% volume
bias seen in cv2-based converters).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Partly also fits with the existing Pull request to update pydicom to the 3x version for reading DICOM files* |
…ches The Unit Tests workflow targeted `runs-on: self-hosted`, but the repo has no registered self-hosted runners, so every job (including the scheduled cron runs on 2026-05-09 and 2026-05-16) sat queued indefinitely until cancelled. Switch to `ubuntu-latest` so the matrix actually executes. The Conformance workflow only fired on `main`, but the default branch is `master`, so commit 7292cd0 (which added the gate) never actually triggered it. Broaden the push filter to `['**']` and drop the PR branch filter so it matches the Unit Tests trigger pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PR's plumbing fix surfaced two pre-existing test failures: 1. test_bmia_stwstrategyhn1 and test_compare_iou both reach out to https://xnat.bmia.nl. They fail under the current xnatpy 0.7.2 because iteration over `subject.experiments` now yields CtSessionData objects instead of ID strings, and they're network-flaky regardless. Mark them `@pytest.mark.network` and exclude them from CI with `-m "not network"`. They still run locally via `pytest -m network`. 2. Python 3.8 went EOL in Oct 2024 and SimpleITK no longer ships cp38 wheels, so CI was hanging in a 30+ minute source build before getting cancelled. Drop 3.8 from the matrix; keep 3.9 / 3.10 / 3.11. The new conformance gate added in 7292cd0 is the proper deterministic test; the BMIA tests are integration tests that don't belong in a fast CI matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After excluding the `network`-marked BMIA tests, the unit-tests workflow
collected zero tests (test_conformance.py self-skips via importorskip when
the conformance extra isn't installed), so pytest exited with code 5 ("no
tests collected") and failed the job.
Add a tiny smoke test that verifies the package and CLI entrypoint import.
Useful on its own as a packaging regression guard and keeps CI green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On CT series with non-uniform Z spacing (e.g. mixed 3/6 mm gaps, common on NSCLC-Radiomics: LUNG1-014/-021/-085/-095/-194/-246), SimpleITK's ImageSeriesReader compresses the per-slice positions into a single averaged ``spacing[2]``. The rasterizer's ``TransformPhysicalPointToContinuousIndex`` then rounds contour Z's against that averaged spacing, missing contour planes on the irregular side of the gap. Empirically that costs ~11-14 of ~73 contour planes on LUNG1-014's lung ROIs. The fix caches each DICOM's ``ImagePositionPatient[2]`` in the same order ITK stacked the series and resolves each contour plane's Z to the nearest cached slice instead. When the IPP tag is missing (anonymized series, corrupt input) the rasterizer cleanly falls back to the legacy rounded-continuous-index path, so behavior on uniform-Z series and inputs without IPP is unchanged. Wired through: - ``DcmInputAdapter.ingest(return_slice_z_positions=True)`` opt-in now also returns the per-slice IPP[2] array; the default single-return signature is preserved for backward compat. - ``DcmPatientCoords2Mask.convert(..., slice_z_positions=...)`` accepts the cached array and consults it when present, falling back otherwise. - The facade ``dcmrtstruct2nii(...)`` threads the array between the two. Tests (``test_nonuniform_z_slice_matching.py``, 5 layers): 1. Uniform-Z fixture: per-slice array matches the IPPs exactly. 2. Non-uniform-Z fixture (mixed 3/6 mm): per-slice array matches the rewritten IPPs; SimpleITK's averaged spacing[2] does NOT match any individual gap (sanity-check that the fixture exercises the bug). 3. End-to-end ``dcmrtstruct2nii(...)`` on a non-uniform-Z synthetic series produces a mask non-empty on every contour-bearing slice (pre-fix this fails on the 6 mm gap slices). 4. ``_resolve_slice_index`` unit test: nearest-IPP wins over the fallback continuous index when both disagree. 5. ``_resolve_slice_index`` fallback: when the array is None, it reduces to ``round(continuous_z)`` -- the legacy path. The synthetic helper (``tests/utils/synthetic.py``) builds a CT series + matching RTSTRUCT via pydicom in ~250 lines, so the test runs with no network access and no optional extras. Cross-version compatible with pydicom 2.x and 3.x via a small ``_save`` shim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nce-trigger Fix CI: use GitHub-hosted runners and trigger conformance on all branches
…hing Fix non-uniform-Z slice matching via nearest-IPP lookup
`zip(..., strict=True)` is Python 3.10+. Build (3.9) failed with
`TypeError: zip() takes no keyword arguments` at synthetic.py:237, causing
all three test_nonuniform_z_slice_matching tests to error in collection.
The same-length precondition is already enforced at synthetic.py:167-168
(`raise ValueError('sop_uids and z_positions must be the same length')`),
so `strict=True` was defensive only. Drop the keyword to restore 3.9
compatibility; the upstream check still catches the mismatch case.
Failed run: https://github.com/brianmanderson/dcmrtstruct2nii/actions/runs/26203235213/job/77097471716
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix Python 3.9 build: drop zip(..., strict=True) in synthetic test util
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
1 participant
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.
Trying to provide an analytical test to evaluate drift in Dicom -> Nifti converters. This contains 7 ROIs (sphere, cylinder, straw, cube, hollow sphere, toroid, ellipse) with metrics of DSC, MSD, and HD95.