DICOM extract: multi-echo plane detection, modality fallback, and 9 smaller fixes - #153
Open
robert-graf wants to merge 49 commits into
Open
robert-graf wants to merge 49 commits into
robert-graf wants to merge 49 commits into
Conversation
Roll the internal-scripts changes together with the new precompute_grid_info_parallel helper in _load_nako_wh, which fans per-file _add_grid_info_to_json calls out to a ProcessPoolExecutor so the sidecar grid cache is populated up front.
The bare `except Exception: print(nii_path)` at the end of _extract_nii_from_dicom still fell through to `return True`. Any unknown conversion error (network, out of disk, dicom2nifti internal bug) was silently treated as success — downstream `_add_grid_info_to_json` and `_split_multi_echo_dixon` then ran on a NIfTI that was never actually written. Now the branch logs a warning, deletes the half-written sidecar JSON, and returns False so the caller stops the pipeline for that series. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Multi-echo series (e.g. Philips mDIX quant) put M echoes at each of N slice positions inside a single ImageType sub-group. Sorted by InstanceNumber, consecutive DICOMs then share ImagePositionPatient, so the direction test computed `0 / 0 = NaN` and `np.allclose(NaN, prev)` false-triggered a stack split — one physical acquisition would be shattered into arbitrary chunks. New helper `_split_by_echo_numbers` picks the smallest `EchoNumbers` bucket for stack detection so each position appears exactly once, then re-attaches the remaining echoes to the detected stacks by `ImagePositionPatient`. Also skips the direction update whenever `norm(delta) < 1e-6`, guarding against residual duplicates and genuinely repeated slices. No-op for single-echo series (0 or 1 distinct EchoNumbers value). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`_from_dicom_to_nii` only tried to split multi-echo output when the BIDS `part` entity resolved to "magnitude" or "phase". Any 4-D volume whose part was mapped to something else via `dixon_mapping` / `parts_mapping` (or none at all) escaped the split and sat on disk as an opaque 4-D file. Since `_split_multi_echo_dixon` is already a no-op on 3-D input, drop the gate and always try to split. Also switch the frame/TE iteration in `_split_multi_echo_dixon` to `zip_strict`. `tes` is padded to `n_echo` upstream, so `zip` is currently safe — but `zip_strict` makes the invariant explicit and traps future regressions instead of silently truncating. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The first `yield dcm_dirs` handed back the raw argument, which is a
`list[Path]` when the caller passed a list. Downstream code (in
`extract_dicom_folder`) then did `str(dicom_path).endswith(".pkl")` on that
list and eventually `_read_dicom_files(list)` — the list-repr string
match ate the guard and `.rglob` blew up with `AttributeError`.
Iterate the normalised `dcm_dirs` list instead, so we always yield `Path`
instances one at a time. Behaviour for the common single-Path case is
unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously `_read_dicom_files` handed every file under a source folder to
`pydicom.dcmread(force=True)` — README.txt, .DS_Store, thumbnail PNGs, JSON
sidecars from earlier extractions, and so on. Each dcmread cost real time
on top-level scanner exports that mix DICOMs with reports and metadata.
New helper `_looks_like_dicom(path)`:
1. Skips files with obviously non-DICOM extensions (json/txt/md/pdf, image
formats, csv/tsv/yaml/html/xml, already-extracted NIfTI / nrrd).
2. Reads the first 132 bytes and accepts the standard "DICM" magic at
offset 128.
3. Falls back to accepting extensionless files (and .dcm / .ima / .dicom),
matching the previous permissive behaviour for Philips/Siemens exports
that ship as `IM000001` and skip the preamble.
Archive extensions (`.zip` / `.tar` / …) are intentionally NOT in the block
list because top-level zips are unpacked by `extract_dicom_folder` before
`_read_dicom_files` sees them, and any residual archive would fail the
DICM-magic check anyway.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`_inc_key` incremented its key while `path_exists(keys)` remained True with no upper bound. A mis-configured `path_exists` callback (e.g. one that never sees the mutation because it captures stale state) would spin forever. Add a hard cap and raise `RuntimeError` with the offending key value instead, so the mistake surfaces immediately. 10 000 tries covers `-a` through `-mnw` letter suffixes — well beyond any realistic filename-collision run. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous one-liner claimed "Defaults to 1 (sequential)" but did not mention that `n_cpu=None` — a common thing to try when reaching for "use all cores" — hits the parallel path with `max_workers=None`, meaning `min(32, os.cpu_count() + 4)` Python threads. Spell all three cases out (1 / >1 / None) and note that this workload is I/O-heavy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two related plane-detection tweaks: 1. The real caller inside `extract_keys_from_json` was passing `hires_threshold=1` while the docstring / function default advertised 0.8. Push the actual call to 0.8 so the behaviour matches the docs (and so submillimetre isotropic MRI still resolves cleanly after the multi-echo pre-filter took the "everything clamps to 1.0" collapse out of the equation). 2. The bare `except Exception: return None` at the end of `get_plane_dicom` swallowed every failure silently. A follow-up `acq-None` filename would then be unattributable. Log a warning with the exception type + message before returning None, guarding the log import itself so the fallback still returns None cleanly if the logger isn't importable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Several common series descriptions were misclassified because the greedy ``.*t2.*`` / ``.*t1.*`` catch-alls matched first (first-match wins). Add specific patterns BEFORE them: * Quantitative maps: MP2RAGE, T2*, R2*, T1map, T2map, T2starmap. * SWI (susceptibility weighted). * Multi-echo VIBE variants for both NAKO Siemens (`ME_vibe_fatquant_*`) and the generic mevibe / fatquant labels. * DIXON auxiliary maps: fatfrac, PDFF, GE Ideal. * Philips localizer aliases the existing pilot/scout entries missed: survey, refscan, smartexam. Also add an explicit ``.*mdixon.*`` alongside the existing ``.*mdix.*`` so both spellings of the Philips protocol name resolve to "dixon". No behaviour change for series that were already correctly classified. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ypart
Three coupled additions so `extract_keys_from_json` stops raising on
non-CT/PET/MR/XA modalities.
* `bids_constants.formats`: add `xray`, `us`, `nm`, `sc`, `photo`,
`endoscopy`, `rtimage`, `rtstruct`, `rtdose`, `rtplan`, `ot`. Without
these entries, `BIDS_FILE.get_changed_bids` refuses the new formats
under strict mode.
* New modality branches in `extract_keys_from_json`. Instead of
`raise NotImplementedError`, dispatch:
- CR/DX/RG/PX/IO/MG → `xray`
- US → `us`
- NM → `nm` (Radiopharmaceutical → `ce` when set)
- SC → `sc`
- OP/XC → `photo`
- ES → `endoscopy`
- RTIMAGE/RTSTRUCT/RTDOSE/RTPLAN → matching lowercase format
- OT → `ot`
- unknown → warn and fall back to `modality.lower()`
so the pipeline can complete instead of aborting mid-tree.
* Two helpers extract the DICOM tags that actually differentiate these
files in a session:
- `_apply_view_keys` lifts `ViewPosition` into `acq` (only overwriting
a None/iso plane) and `ImageLaterality`/`Laterality` into `part`,
with ophthalmic OS/OD normalised to L/R.
- `_apply_bodypart_key` lifts `BodyPartExamined` into `desc` when
empty, matching common values (ABDOMEN, PELVIS, CHEST, HEAD…).
Called from CT, PET, and every new modality branch so files that only
differ by view (MG four-view screening), laterality (ophthalmic),
tracer (NM), or body region no longer collapse onto identical BIDS
filenames.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Non-imaging DICOMs (RTSTRUCT / RTDOSE / SR / presentation states, and any other object without a spatial header) legally lack `ImagePositionPatient` / `ImageOrientationPatient`. `dicom2nifti.common.create_affine` then raises `AttributeError`, and the previous warning branch surfaced that as noise on every conversion of a mixed source tree. Split the exception handler: `AttributeError` returns None silently, every other exception still warns so real plane-detection bugs stay visible. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Benchmark comparisonSpeed (wall time per call)baseline Showing the 5 most-changed measurements per case; the rest are collapsed. Rows with a baseline below 3 ms are tagged
|
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_rescale_seg |
3.05 ±0.02 | 3.15 ±0.11 | +3.4% | 0.075 |
nii_filter_connected_components |
4.33 ±0.07 | 4.20 ±0.08 | -3.1% | 0.045 |
nii_erode_msk |
6.06 ±0.05 | 6.20 ±0.16 | +2.3% | 0.134 |
nii_fill_holes |
4.43 ±0.03 | 4.53 ±0.05 | +2.2% | 0.009 |
nii_load_img |
10.58 ±0.13 | 10.81 ±0.18 | +2.1% | 0.085 |
… 31 more measurements
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
poi_calc_poi_from_subreg_vert |
12.64 ±0.25 | 12.48 ±0.12 | -1.3% | 0.586 |
nii_dilate_msk |
124.22 ±0.27 | 125.79 ±1.07 | +1.3% | 0.050 |
nii_save |
11.44 ±0.08 | 11.50 ±0.13 | +0.5% | 0.227 |
poi_calc_centroids_nocrop |
4.16 ±0.04 | 4.15 ±0.07 | -0.4% | 0.558 |
nii_rescale |
39.73 ±0.10 | 39.85 ±0.23 | +0.3% | 0.717 |
nii_resample_from_to |
41.64 ±0.29 | 41.70 ±0.35 | +0.1% | 0.871 |
metric_voxels |
250463.00 ±0.00 | 250463.00 ±0.00 | +0.0% | — |
metric_labels |
3.00 ±0.00 | 3.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
14.51 ±0.00 | 14.51 ±0.00 | +0.0% | — |
nii_extract_label |
0.34 ±0.05 | 0.42 ±0.02 | +26.2% (noise) | 0.069 |
nii_get_array |
0.04 ±0.01 | 0.03 ±0.00 | -18.4% (noise) | 0.115 |
nii_set_dtype |
0.47 ±0.04 | 0.40 ±0.04 | -15.7% (noise) | 0.155 |
poi_map_labels |
0.15 ±0.01 | 0.17 ±0.02 | +14.3% (noise) | 0.032 |
nii_apply_crop |
0.56 ±0.01 | 0.62 ±0.04 | +11.9% (noise) | 0.019 |
poi_local_to_global_arr |
0.30 ±0.02 | 0.27 ±0.03 | -11.7% (noise) | 0.351 |
poi_reorient |
0.38 ±0.03 | 0.34 ±0.02 | -11.0% (noise) | 0.085 |
nii_load_seg |
2.62 ±0.04 | 2.90 ±0.10 | +10.7% (noise) | 0.001 |
poi_to_global |
0.17 ±0.02 | 0.19 ±0.01 | +8.8% (noise) | 0.270 |
nii_compute_crop |
0.29 ±0.03 | 0.31 ±0.01 | +6.7% (noise) | 0.337 |
poi_rescale |
0.19 ±0.01 | 0.17 ±0.01 | -5.9% (noise) | 0.249 |
poi_load |
0.92 ±0.04 | 0.88 ±0.03 | -3.5% (noise) | 0.493 |
nii_volumes |
1.88 ±0.04 | 1.82 ±0.03 | -3.1% (noise) | 0.078 |
nii_pad_to |
0.63 ±0.07 | 0.61 ±0.07 | -2.6% (noise) | 0.520 |
nii_map_labels |
0.90 ±0.04 | 0.92 ±0.02 | +2.5% (noise) | 0.803 |
nii_get_connected_components |
2.17 ±0.04 | 2.22 ±0.06 | +2.3% (noise) | 0.240 |
poi_calc_centroids |
2.39 ±0.01 | 2.34 ±0.05 | -2.0% (noise) | 0.031 |
poi_save |
0.37 ±0.02 | 0.37 ±0.04 | +1.2% (noise) | 0.193 |
nii_center_of_masses |
1.68 ±0.03 | 1.69 ±0.03 | +0.8% (noise) | 0.618 |
nii_reorient |
0.79 ±0.07 | 0.80 ±0.03 | +0.7% (noise) | 0.499 |
poi_resample_from_to |
0.58 ±0.03 | 0.57 ±0.03 | -0.4% (noise) | 0.616 |
nii_unique |
0.64 ±0.04 | 0.64 ±0.02 | +0.2% (noise) | 0.446 |
ct_2d — shape (73, 47, 1)
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_resample_from_to |
7.54 ±0.06 | 7.37 ±0.06 | -2.3% | 0.000 |
nii_rescale |
5.50 ±0.17 | 5.41 ±0.12 | -1.6% | 0.186 |
nii_dilate_msk |
14.64 ±0.07 | 14.53 ±0.05 | -0.7% | 0.013 |
metric_voxels |
3431.00 ±0.00 | 3431.00 ±0.00 | +0.0% | — |
metric_labels |
3.00 ±0.00 | 3.00 ±0.00 | +0.0% | — |
… 30 more measurements
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
metric_foreground_pct |
37.60 ±0.00 | 37.60 ±0.00 | +0.0% | — |
nii_pad_to |
0.59 ±0.02 | 0.53 ±0.01 | -9.9% (noise) | 0.001 |
nii_get_array |
0.02 ±0.00 | 0.02 ±0.00 | -9.4% (noise) | 0.233 |
poi_map_labels |
0.20 ±0.01 | 0.19 ±0.01 | -6.6% (noise) | 0.165 |
poi_to_global |
0.25 ±0.01 | 0.23 ±0.01 | -6.1% (noise) | 0.177 |
nii_unique |
0.12 ±0.01 | 0.12 ±0.01 | -4.4% (noise) | 0.417 |
nii_set_dtype |
0.41 ±0.04 | 0.40 ±0.02 | -4.3% (noise) | 0.483 |
poi_save |
0.49 ±0.01 | 0.47 ±0.02 | -4.0% (noise) | 0.489 |
nii_volumes |
0.27 ±0.01 | 0.28 ±0.01 | +3.5% (noise) | 0.238 |
poi_rescale |
0.26 ±0.01 | 0.25 ±0.01 | -3.4% (noise) | 0.192 |
nii_apply_crop |
0.67 ±0.02 | 0.69 ±0.04 | +2.9% (noise) | 0.153 |
nii_rescale_seg |
0.77 ±0.02 | 0.79 ±0.02 | +2.8% (noise) | 0.291 |
poi_local_to_global_arr |
0.34 ±0.02 | 0.33 ±0.01 | -2.5% (noise) | 0.275 |
poi_reorient |
0.44 ±0.01 | 0.45 ±0.03 | +2.3% (noise) | 0.843 |
poi_resample_from_to |
0.73 ±0.01 | 0.71 ±0.03 | -2.3% (noise) | 0.628 |
nii_compute_crop |
0.14 ±0.01 | 0.14 ±0.00 | -2.2% (noise) | 0.580 |
nii_erode_msk |
0.98 ±0.01 | 1.00 ±0.06 | +2.0% (noise) | 0.196 |
nii_map_labels |
0.48 ±0.02 | 0.49 ±0.03 | +2.0% (noise) | 0.168 |
nii_load_seg |
1.66 ±0.03 | 1.70 ±0.08 | +1.9% (noise) | 0.236 |
nii_center_of_masses |
0.21 ±0.01 | 0.21 ±0.01 | +1.7% (noise) | 0.273 |
poi_load |
1.10 ±0.05 | 1.09 ±0.03 | -1.6% (noise) | 0.346 |
poi_calc_centroids |
0.91 ±0.04 | 0.92 ±0.04 | +1.4% (noise) | 0.540 |
poi_calc_centroids_nocrop |
1.13 ±0.03 | 1.14 ±0.03 | +0.9% (noise) | 0.984 |
nii_load_img |
1.49 ±0.05 | 1.50 ±0.03 | +0.8% (noise) | 0.616 |
nii_filter_connected_components |
0.82 ±0.02 | 0.83 ±0.02 | +0.8% (noise) | 0.886 |
nii_extract_label |
0.44 ±0.01 | 0.44 ±0.03 | +0.8% (noise) | 0.856 |
nii_fill_holes |
0.83 ±0.03 | 0.82 ±0.02 | -0.7% (noise) | 0.612 |
nii_save |
1.35 ±0.08 | 1.34 ±0.03 | -0.2% (noise) | 0.248 |
nii_reorient |
0.83 ±0.03 | 0.83 ±0.04 | -0.2% (noise) | 0.390 |
nii_get_connected_components |
0.63 ±0.03 | 0.63 ±0.02 | -0.1% (noise) | 0.398 |
mri_3d — shape (68, 52, 67)
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
poi_calc_centroids_nocrop |
8.85 ±0.09 | 8.46 ±0.63 | -4.4% | 0.943 |
nii_save |
9.69 ±0.24 | 9.27 ±0.06 | -4.3% | 0.023 |
nii_fill_holes |
5.46 ±0.06 | 5.33 ±0.07 | -2.4% | 0.007 |
nii_rescale |
38.05 ±0.43 | 37.36 ±0.24 | -1.8% | 0.029 |
nii_dilate_msk |
160.99 ±0.26 | 162.58 ±4.86 | +1.0% | 0.249 |
… 31 more measurements
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_resample_from_to |
39.80 ±0.66 | 39.44 ±0.40 | -0.9% | 0.290 |
poi_calc_poi_from_subreg_vert |
11.95 ±0.11 | 11.85 ±0.12 | -0.8% | 0.267 |
nii_filter_connected_components |
4.76 ±0.06 | 4.73 ±0.09 | -0.7% | 0.454 |
nii_load_img |
8.79 ±0.30 | 8.73 ±0.07 | -0.6% | 0.189 |
nii_erode_msk |
7.83 ±0.14 | 7.80 ±0.13 | -0.4% | 0.396 |
nii_rescale_seg |
3.24 ±0.08 | 3.24 ±0.62 | +0.1% | 0.341 |
metric_voxels |
236912.00 ±0.00 | 236912.00 ±0.00 | +0.0% | — |
metric_labels |
8.00 ±0.00 | 8.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
18.21 ±0.00 | 18.21 ±0.00 | +0.0% | — |
nii_get_array |
0.06 ±0.01 | 0.05 ±0.00 | -16.8% (noise) | 0.024 |
poi_load |
2.44 ±0.10 | 2.18 ±0.09 | -10.4% (noise) | 0.017 |
poi_local_to_global_arr |
0.38 ±0.01 | 0.34 ±0.01 | -10.3% (noise) | 0.000 |
nii_reorient |
0.93 ±0.02 | 0.85 ±0.03 | -8.7% (noise) | 0.000 |
nii_pad_to |
0.80 ±0.04 | 0.75 ±0.03 | -6.3% (noise) | 0.153 |
poi_map_labels |
0.26 ±0.02 | 0.25 ±0.01 | -6.1% (noise) | 0.224 |
poi_save |
0.57 ±0.02 | 0.54 ±0.03 | -5.9% (noise) | 0.008 |
nii_set_dtype |
0.56 ±0.01 | 0.54 ±0.02 | -5.2% (noise) | 0.010 |
poi_to_global |
0.28 ±0.01 | 0.27 ±0.01 | -5.1% (noise) | 0.011 |
poi_rescale |
0.31 ±0.02 | 0.29 ±0.01 | -5.1% (noise) | 0.057 |
nii_unique |
0.68 ±0.01 | 0.65 ±0.00 | -4.6% (noise) | 0.003 |
poi_resample_from_to |
0.81 ±0.03 | 0.78 ±0.02 | -3.9% (noise) | 0.042 |
nii_map_labels |
1.08 ±0.03 | 1.04 ±0.02 | -3.2% (noise) | 0.084 |
poi_reorient |
0.49 ±0.03 | 0.48 ±0.02 | -2.5% (noise) | 0.162 |
nii_extract_label |
0.45 ±0.02 | 0.46 ±0.02 | +1.8% (noise) | 0.498 |
nii_load_seg |
2.82 ±0.12 | 2.87 ±0.05 | +1.6% (noise) | 0.934 |
nii_get_connected_components |
2.24 ±0.08 | 2.21 ±0.03 | -1.4% (noise) | 0.106 |
poi_calc_centroids |
2.88 ±0.07 | 2.84 ±0.10 | -1.3% (noise) | 0.565 |
nii_center_of_masses |
1.99 ±0.02 | 1.97 ±0.06 | -0.9% (noise) | 0.577 |
nii_compute_crop |
0.34 ±0.01 | 0.34 ±0.02 | +0.8% (noise) | 0.791 |
nii_volumes |
1.96 ±0.01 | 1.98 ±0.01 | +0.7% (noise) | 0.088 |
nii_apply_crop |
0.74 ±0.04 | 0.75 ±0.04 | +0.7% (noise) | 0.666 |
mri_2d — shape (68, 52, 1)
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_rescale |
5.45 ±0.04 | 5.29 ±0.07 | -2.9% | 0.001 |
nii_resample_from_to |
7.53 ±0.08 | 7.41 ±0.07 | -1.6% | 0.107 |
nii_dilate_msk |
19.48 ±0.07 | 19.41 ±0.12 | -0.4% | 0.113 |
metric_voxels |
3536.00 ±0.00 | 3536.00 ±0.00 | +0.0% | — |
metric_labels |
7.00 ±0.00 | 7.00 ±0.00 | +0.0% | — |
… 30 more measurements
| Measurement | baseline ms (median ±½·range) | head ms (median ±½·range) | Δ % | p |
|---|---|---|---|---|
metric_foreground_pct |
50.17 ±0.00 | 50.17 ±0.00 | +0.0% | — |
poi_map_labels |
0.24 ±0.01 | 0.22 ±0.01 | -9.5% (noise) | 0.010 |
nii_map_labels |
0.48 ±0.02 | 0.51 ±0.02 | +6.2% (noise) | 0.015 |
poi_save |
0.54 ±0.24 | 0.57 ±0.58 | +6.1% (noise) | 0.568 |
nii_volumes |
0.49 ±0.03 | 0.46 ±0.01 | -5.7% (noise) | 0.120 |
nii_extract_label |
0.43 ±0.02 | 0.41 ±0.01 | -5.6% (noise) | 0.072 |
poi_load |
2.05 ±0.11 | 1.95 ±0.07 | -5.1% (noise) | 0.062 |
nii_unique |
0.12 ±0.00 | 0.12 ±0.00 | +4.1% (noise) | 0.080 |
nii_compute_crop |
0.15 ±0.01 | 0.14 ±0.01 | -4.0% (noise) | 0.324 |
poi_resample_from_to |
0.77 ±0.03 | 0.75 ±0.02 | -3.6% (noise) | 0.082 |
poi_to_global |
0.26 ±0.02 | 0.26 ±0.01 | -3.5% (noise) | 0.243 |
nii_get_array |
0.02 ±0.00 | 0.02 ±0.00 | -3.1% (noise) | 0.337 |
nii_set_dtype |
0.40 ±0.02 | 0.41 ±0.00 | +3.1% (noise) | 0.886 |
nii_get_connected_components |
0.61 ±0.02 | 0.62 ±0.02 | +2.8% (noise) | 0.363 |
nii_load_img |
1.51 ±0.05 | 1.47 ±0.07 | -2.6% (noise) | 0.287 |
poi_calc_centroids |
1.17 ±0.06 | 1.14 ±0.02 | -2.5% (noise) | 0.161 |
nii_apply_crop |
0.65 ±0.01 | 0.67 ±0.02 | +2.2% (noise) | 0.508 |
poi_reorient |
0.48 ±0.02 | 0.47 ±0.02 | -2.1% (noise) | 0.650 |
nii_save |
1.38 ±0.05 | 1.35 ±0.02 | -2.1% (noise) | 0.065 |
nii_pad_to |
0.55 ±0.02 | 0.54 ±0.03 | -1.8% (noise) | 0.799 |
nii_rescale_seg |
0.77 ±0.02 | 0.76 ±0.03 | -1.7% (noise) | 0.177 |
nii_filter_connected_components |
0.86 ±0.04 | 0.85 ±0.03 | -1.5% (noise) | 0.482 |
nii_fill_holes |
1.11 ±0.01 | 1.09 ±0.03 | -1.5% (noise) | 0.334 |
nii_erode_msk |
1.30 ±0.03 | 1.29 ±0.04 | -0.9% (noise) | 0.636 |
nii_reorient |
0.83 ±0.03 | 0.84 ±0.02 | +0.7% (noise) | 0.674 |
poi_calc_centroids_nocrop |
1.44 ±0.05 | 1.43 ±0.05 | -0.6% (noise) | 0.455 |
poi_rescale |
0.28 ±0.01 | 0.28 ±0.01 | -0.6% (noise) | 0.282 |
poi_local_to_global_arr |
0.33 ±0.01 | 0.33 ±0.02 | +0.5% (noise) | 0.513 |
nii_center_of_masses |
0.41 ±0.01 | 0.42 ±0.03 | +0.2% (noise) | 0.909 |
nii_load_seg |
1.69 ±0.05 | 1.69 ±0.07 | +0.0% (noise) | 0.551 |
Gate: a measurement fails when the baseline is ≥ 1 ms, the median grows by ≥ 50%, and Welch's t-test gives p < 0.05. metric_* rows are context only.
Memory (peak RSS growth per call)
baseline 32c52dd vs head 32c52dd · python 3.11.16 · 5 repeats + 1 warmup · sampler proc-status, isolation fork · measurement floor ≈ 0.90 MiB
Showing the 5 most-changed measurements per case; the rest are collapsed. Rows with a baseline below 3 MiB are tagged (noise) — runner jitter on those swamps any real change.
ct_3d — shape (73, 47, 73)
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_save |
3.67 ±0.00 | 3.71 ±0.00 | +1.0% | — |
nii_apply_crop |
5.77 ±0.00 | 5.80 ±0.00 | +0.6% | — |
nii_rescale_seg |
5.84 ±0.00 | 5.88 ±0.00 | +0.6% | — |
nii_rescale |
6.03 ±0.00 | 6.07 ±0.00 | +0.6% | — |
nii_resample_from_to |
6.16 ±0.00 | 6.19 ±0.00 | +0.6% | — |
… 31 more measurements
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_get_connected_components |
6.52 ±0.00 | 6.55 ±0.00 | +0.5% | — |
nii_filter_connected_components |
6.77 ±0.00 | 6.80 ±0.00 | +0.5% | — |
poi_calc_poi_from_subreg_vert |
7.01 ±0.00 | 7.05 ±0.00 | +0.5% | — |
nii_erode_msk |
5.86 ±0.00 | 5.88 ±0.00 | +0.3% | — |
nii_load_img |
3.53 ±0.00 | 3.52 ±0.00 | -0.2% | — |
nii_load_seg |
5.91 ±0.00 | 5.91 ±0.00 | -0.1% | — |
poi_to_global |
3.08 ±0.00 | 3.08 ±0.00 | -0.1% | — |
nii_fill_holes |
6.22 ±0.00 | 6.21 ±0.00 | -0.1% | — |
nii_center_of_masses |
3.27 ±0.00 | 3.27 ±0.00 | -0.1% | — |
nii_volumes |
3.52 ±0.00 | 3.52 ±0.00 | -0.1% | — |
poi_reorient |
3.83 ±0.00 | 3.83 ±0.00 | -0.1% | — |
poi_resample_from_to |
5.18 ±0.00 | 5.17 ±0.00 | -0.1% | — |
nii_set_dtype |
5.18 ±0.00 | 5.18 ±0.00 | -0.1% | — |
nii_extract_label |
5.30 ±0.00 | 5.30 ±0.00 | -0.1% | — |
nii_pad_to |
5.36 ±0.00 | 5.36 ±0.00 | -0.1% | — |
nii_map_labels |
5.43 ±0.00 | 5.43 ±0.00 | -0.1% | — |
nii_reorient |
5.80 ±0.00 | 5.80 ±0.00 | -0.1% | — |
poi_calc_centroids_nocrop |
5.86 ±0.00 | 5.86 ±0.00 | -0.1% | — |
nii_dilate_msk |
5.87 ±0.00 | 5.86 ±0.00 | -0.1% | — |
poi_calc_centroids |
5.99 ±0.00 | 5.99 ±0.00 | -0.1% | — |
metric_voxels |
250463.00 ±0.00 | 250463.00 ±0.00 | +0.0% | — |
metric_labels |
3.00 ±0.00 | 3.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
14.51 ±0.00 | 14.51 ±0.00 | +0.0% | — |
poi_rescale |
2.61 ±0.00 | 2.60 ±0.00 | -0.1% (noise) | — |
nii_compute_crop |
2.90 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
poi_local_to_global_arr |
2.96 ±0.00 | 2.95 ±0.00 | -0.1% (noise) | — |
nii_unique |
2.96 ±0.00 | 2.96 ±0.00 | -0.1% (noise) | — |
nii_get_array |
1.65 ±0.00 | 1.65 ±0.00 | +0.0% (noise) | — |
poi_save |
1.40 ±0.00 | 1.40 ±0.00 | +0.0% (noise) | — |
poi_map_labels |
1.77 ±0.00 | 1.77 ±0.00 | +0.0% (noise) | — |
poi_load |
1.61 ±0.00 | 1.61 ±0.00 | +0.0% (noise) | — |
ct_2d — shape (73, 47, 1)
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_save |
3.61 ±0.00 | 3.64 ±0.00 | +1.0% | — |
nii_apply_crop |
5.70 ±0.00 | 5.74 ±0.00 | +0.6% | — |
nii_rescale_seg |
5.78 ±0.00 | 5.82 ±0.00 | +0.6% | — |
nii_rescale |
5.97 ±0.00 | 6.00 ±0.00 | +0.6% | — |
nii_resample_from_to |
6.09 ±0.00 | 6.13 ±0.00 | +0.6% | — |
… 30 more measurements
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_get_connected_components |
6.45 ±0.00 | 6.49 ±0.00 | +0.5% | — |
nii_filter_connected_components |
6.70 ±0.00 | 6.74 ±0.00 | +0.5% | — |
nii_erode_msk |
5.80 ±0.00 | 5.82 ±0.00 | +0.3% | — |
nii_load_img |
3.47 ±0.00 | 3.46 ±0.00 | -0.2% | — |
nii_load_seg |
5.85 ±0.00 | 5.84 ±0.00 | -0.1% | — |
poi_to_global |
3.02 ±0.00 | 3.02 ±0.00 | -0.1% | — |
nii_fill_holes |
6.16 ±0.00 | 6.15 ±0.00 | -0.1% | — |
nii_center_of_masses |
3.21 ±0.00 | 3.21 ±0.00 | -0.1% | — |
nii_volumes |
3.46 ±0.00 | 3.46 ±0.00 | -0.1% | — |
poi_reorient |
3.77 ±0.00 | 3.77 ±0.00 | -0.1% | — |
poi_resample_from_to |
5.11 ±0.00 | 5.11 ±0.00 | -0.1% | — |
nii_set_dtype |
5.12 ±0.00 | 5.11 ±0.00 | -0.1% | — |
nii_extract_label |
5.24 ±0.00 | 5.24 ±0.00 | -0.1% | — |
nii_pad_to |
5.30 ±0.00 | 5.30 ±0.00 | -0.1% | — |
nii_map_labels |
5.37 ±0.00 | 5.36 ±0.00 | -0.1% | — |
nii_reorient |
5.74 ±0.00 | 5.74 ±0.00 | -0.1% | — |
nii_dilate_msk |
5.80 ±0.00 | 5.80 ±0.00 | -0.1% | — |
poi_calc_centroids_nocrop |
5.86 ±0.00 | 5.86 ±0.00 | -0.1% | — |
poi_calc_centroids |
5.93 ±0.00 | 5.93 ±0.00 | -0.1% | — |
metric_voxels |
3431.00 ±0.00 | 3431.00 ±0.00 | +0.0% | — |
metric_labels |
3.00 ±0.00 | 3.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
37.60 ±0.00 | 37.60 ±0.00 | +0.0% | — |
poi_rescale |
2.54 ±0.00 | 2.54 ±0.00 | -0.2% (noise) | — |
nii_compute_crop |
2.84 ±0.00 | 2.83 ±0.00 | -0.1% (noise) | — |
poi_local_to_global_arr |
2.89 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_unique |
2.90 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_get_array |
1.59 ±0.00 | 1.59 ±0.00 | +0.0% (noise) | — |
poi_save |
1.34 ±0.00 | 1.34 ±0.00 | +0.0% (noise) | — |
poi_map_labels |
1.71 ±0.00 | 1.71 ±0.00 | +0.0% (noise) | — |
poi_load |
1.55 ±0.00 | 1.55 ±0.00 | +0.0% (noise) | — |
mri_3d — shape (68, 52, 67)
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_save |
3.61 ±0.00 | 3.64 ±0.00 | +1.0% | — |
nii_apply_crop |
5.70 ±0.00 | 5.74 ±0.00 | +0.6% | — |
nii_rescale_seg |
5.78 ±0.00 | 5.82 ±0.00 | +0.6% | — |
nii_rescale |
5.97 ±0.00 | 6.00 ±0.00 | +0.6% | — |
nii_resample_from_to |
6.09 ±0.00 | 6.13 ±0.00 | +0.6% | — |
… 31 more measurements
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_get_connected_components |
6.45 ±0.00 | 6.49 ±0.00 | +0.5% | — |
nii_filter_connected_components |
6.70 ±0.00 | 6.74 ±0.00 | +0.5% | — |
poi_calc_poi_from_subreg_vert |
7.01 ±0.00 | 7.05 ±0.00 | +0.5% | — |
nii_erode_msk |
5.80 ±0.00 | 5.82 ±0.00 | +0.3% | — |
nii_load_img |
3.53 ±0.00 | 3.52 ±0.00 | -0.2% | — |
nii_load_seg |
5.85 ±0.00 | 5.84 ±0.00 | -0.1% | — |
poi_to_global |
3.02 ±0.00 | 3.02 ±0.00 | -0.1% | — |
nii_fill_holes |
6.16 ±0.00 | 6.15 ±0.00 | -0.1% | — |
nii_center_of_masses |
3.21 ±0.00 | 3.21 ±0.00 | -0.1% | — |
nii_volumes |
3.46 ±0.00 | 3.46 ±0.00 | -0.1% | — |
poi_reorient |
3.77 ±0.00 | 3.77 ±0.00 | -0.1% | — |
poi_resample_from_to |
5.11 ±0.00 | 5.11 ±0.00 | -0.1% | — |
nii_set_dtype |
5.18 ±0.00 | 5.18 ±0.00 | -0.1% | — |
nii_extract_label |
5.24 ±0.00 | 5.24 ±0.00 | -0.1% | — |
nii_pad_to |
5.30 ±0.00 | 5.30 ±0.00 | -0.1% | — |
nii_map_labels |
5.37 ±0.00 | 5.36 ±0.00 | -0.1% | — |
nii_reorient |
5.74 ±0.00 | 5.74 ±0.00 | -0.1% | — |
poi_calc_centroids_nocrop |
5.80 ±0.00 | 5.80 ±0.00 | -0.1% | — |
nii_dilate_msk |
5.80 ±0.00 | 5.80 ±0.00 | -0.1% | — |
poi_calc_centroids |
5.93 ±0.00 | 5.93 ±0.00 | -0.1% | — |
metric_voxels |
236912.00 ±0.00 | 236912.00 ±0.00 | +0.0% | — |
metric_labels |
8.00 ±0.00 | 8.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
18.21 ±0.00 | 18.21 ±0.00 | +0.0% | — |
poi_rescale |
2.54 ±0.00 | 2.54 ±0.00 | -0.2% (noise) | — |
nii_compute_crop |
2.84 ±0.00 | 2.83 ±0.00 | -0.1% (noise) | — |
poi_local_to_global_arr |
2.89 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_unique |
2.90 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_get_array |
1.59 ±0.00 | 1.59 ±0.00 | +0.0% (noise) | — |
poi_save |
1.40 ±0.00 | 1.40 ±0.00 | +0.0% (noise) | — |
poi_map_labels |
1.71 ±0.00 | 1.71 ±0.00 | +0.0% (noise) | — |
poi_load |
1.55 ±0.00 | 1.55 ±0.00 | +0.0% (noise) | — |
mri_2d — shape (68, 52, 1)
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_save |
3.61 ±0.00 | 3.64 ±0.00 | +1.0% | — |
nii_apply_crop |
5.70 ±0.00 | 5.74 ±0.00 | +0.6% | — |
nii_rescale_seg |
5.78 ±0.00 | 5.82 ±0.00 | +0.6% | — |
nii_rescale |
5.97 ±0.00 | 6.00 ±0.00 | +0.6% | — |
nii_resample_from_to |
6.09 ±0.00 | 6.13 ±0.00 | +0.6% | — |
… 30 more measurements
| Measurement | baseline MiB (median ±½·range) | head MiB (median ±½·range) | Δ % | p |
|---|---|---|---|---|
nii_get_connected_components |
6.45 ±0.00 | 6.49 ±0.00 | +0.5% | — |
nii_filter_connected_components |
6.70 ±0.00 | 6.74 ±0.00 | +0.5% | — |
nii_erode_msk |
5.80 ±0.00 | 5.82 ±0.00 | +0.3% | — |
nii_load_img |
3.53 ±0.00 | 3.52 ±0.00 | -0.2% | — |
nii_load_seg |
5.85 ±0.00 | 5.84 ±0.00 | -0.1% | — |
poi_to_global |
3.02 ±0.00 | 3.02 ±0.00 | -0.1% | — |
nii_fill_holes |
6.16 ±0.00 | 6.15 ±0.00 | -0.1% | — |
nii_center_of_masses |
3.21 ±0.00 | 3.21 ±0.00 | -0.1% | — |
nii_volumes |
3.46 ±0.00 | 3.46 ±0.00 | -0.1% | — |
poi_reorient |
3.77 ±0.00 | 3.77 ±0.00 | -0.1% | — |
poi_resample_from_to |
5.11 ±0.00 | 5.11 ±0.00 | -0.1% | — |
nii_set_dtype |
5.18 ±0.00 | 5.18 ±0.00 | -0.1% | — |
nii_extract_label |
5.24 ±0.00 | 5.24 ±0.00 | -0.1% | — |
nii_pad_to |
5.30 ±0.00 | 5.30 ±0.00 | -0.1% | — |
nii_map_labels |
5.37 ±0.00 | 5.36 ±0.00 | -0.1% | — |
nii_reorient |
5.74 ±0.00 | 5.74 ±0.00 | -0.1% | — |
nii_dilate_msk |
5.80 ±0.00 | 5.80 ±0.00 | -0.1% | — |
poi_calc_centroids_nocrop |
5.86 ±0.00 | 5.86 ±0.00 | -0.1% | — |
poi_calc_centroids |
5.93 ±0.00 | 5.93 ±0.00 | -0.1% | — |
metric_voxels |
3536.00 ±0.00 | 3536.00 ±0.00 | +0.0% | — |
metric_labels |
7.00 ±0.00 | 7.00 ±0.00 | +0.0% | — |
metric_foreground_pct |
50.17 ±0.00 | 50.17 ±0.00 | +0.0% | — |
poi_rescale |
2.54 ±0.00 | 2.54 ±0.00 | -0.2% (noise) | — |
nii_compute_crop |
2.84 ±0.00 | 2.83 ±0.00 | -0.1% (noise) | — |
poi_local_to_global_arr |
2.89 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_unique |
2.90 ±0.00 | 2.89 ±0.00 | -0.1% (noise) | — |
nii_get_array |
1.59 ±0.00 | 1.59 ±0.00 | +0.0% (noise) | — |
poi_save |
1.40 ±0.00 | 1.40 ±0.00 | +0.0% (noise) | — |
poi_map_labels |
1.71 ±0.00 | 1.71 ±0.00 | +0.0% (noise) | — |
poi_load |
1.55 ±0.00 | 1.55 ±0.00 | +0.0% (noise) | — |
Gate: a measurement fails when the baseline is ≥ 1 MiB, the median grows by ≥ 50%, and Welch's t-test gives p < 0.05. metric_* rows are context only.
`_extract_nii_from_dicom` had `return True` inside the `len == 1` branch regardless of whether `dicom_to_nifti_multiframe` actually wrote a file. Any single-DICOM series without `pixel_array` — Presentation State, Key Object Selection, Registration, Fiducials, some RT objects — was reported as successfully converted, and the caller then crashed inside `_add_grid_info_to_json` with `FileNotFoundError` on a NIfTI that was never produced. Move `return True` inside the pixel-data guard; when the guard fails, delete the half-written sidecar and return False so the caller stops the pipeline for that series. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`test_name_conflict` opened the existing file with `encoding="utf-8"` and handed the bytes straight to `json.load`. When an interrupted extraction or an unrelated same-name file left non-UTF-8 / non-JSON content on disk (seen in the wild as `UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa4 in position 1`), the exception propagated out of `_generate_bids_path` and killed the whole series conversion. Catch UnicodeDecodeError / JSONDecodeError / OSError and return True (= conflict) so the caller's `_inc_key` path assigns a fresh, non- colliding filename instead of crashing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously the modality fallback in `extract_keys_from_json` routed every non-CT/PET/MR/XA modality to a `.nii.gz` output — including Presentation State (PR), Key Object Selection (KO), Registration (REG), Fiducials (FID), Real World Value Map (RWV), Plan (PLAN), Slide Stainer (STAIN), waveform types (RESP / HD / ECG / EPS) and ophthalmic measurements (AR / KER / LEN / VA / OPV / OPM). None of those carry a NIfTI-shaped pixel volume, so the caller then either wrote a corrupt file or crashed in the follow-up grid step. Route them through the same `.txt` sidecar path already used for SR: lower-cased modality tag becomes the BIDS format, `SeriesDescription` becomes `desc`, and the ImageType-driven `part` / view / laterality helpers upstream still enrich the filename. Also drop the now-unused `simp_json` argument from `_apply_view_keys` so ruff stops complaining about ARG001. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previous behaviour deleted the sidecar JSON when `_extract_nii_from_dicom`
hit a single DICOM without `pixel_array`. That threw away every
non-pixel DICOM tag (which the JSON already holds in full) for anything
that didn't match the explicit modality routing (PR/KO/REG/…) — malformed
CT/MR series, unknown modalities, DICOMs with `pixel_array.shape < 2`.
Now the branch:
* Attempts to write a `.txt` companion via `_extract_txt_from_dicom` so
a DICOM that only carries a ContentSequence still leaves its textual
payload on disk. An empty result is removed to avoid noise.
* Keeps the JSON header sidecar so downstream inspection still has
every tag from the source DICOM.
* Returns False so the caller skips `_add_grid_info_to_json` on the
nonexistent NIfTI.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…g around it The previous fix caught UnicodeDecodeError / JSONDecodeError / OSError and returned True (= "conflict"), which sent the caller through `_inc_key` to find a fresh non-colliding filename. In practice the unreadable file is almost always a half-written sidecar from an earlier crashed extraction, not an unrelated file the extractor should tiptoe around — so the `_inc_key` path only piled up `_sequ-<n>-a` copies next to the broken original. Now we unlink the corrupt file, log a warning, and return False so the caller writes fresh content over the slot. If the unlink itself fails (permissions, read-only filesystem) we fall back to the rename path so the caller still succeeds. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`_collect_text` only produced output for DICOMs with a `ContentSequence` (Structured Reports). For every other non-imaging modality routed through the `.txt` path — Presentation State, Key Object Selection, Registration, Fiducials, waveforms, ophthalmic measurements — the walker returned nothing and left an empty `_pr.txt` / `_ko.txt` / `_reg.txt` on disk. Fall back to iterating every DICOM element (via `Dataset.iterall`) and appending the tag/name/VR/value line pydicom already knows how to render. Skip the PixelData group so the sidecar stays small; ultimate fallback is `str(ds).splitlines()` if `iterall` raises for any reason. Verified against a synthetic PR-modality dataset: previously 0 lines, now the full header (Modality, SeriesDescription, PatientName, …). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Presentation State (GSPS) DICOMs — SOP Class 1.2.840.10008.5.1.4.1.11.1,
BIDS format ``pr`` after the modality fallback — hold no pixel data.
They only carry Referenced SOP Instance UIDs into other series plus
viewer window/level, zoom, pan and annotation presets. For an automated
segmentation pipeline they are pure noise: `_read_dicom_files` still
groups them per-echo/per-SeriesInstanceUID via `_inc_key`, which meant a
single scanner session shipped ~10 `_pr.txt` files per subject next to
the actual imaging.
Add a `skip_formats: set[str]` parameter to both `_from_dicom_to_nii`
and `extract_dicom_folder`, default `{"pr"}`, and drop matching series
after the BIDS format is resolved but before any JSON / NIfTI / sidecar
is written. Callers can pass `set()` to keep every format, or add more
formats to also drop e.g. Key Object Selection (`"ko"`) or Registration
(`"reg"`).
Behaviour otherwise unchanged for every currently-imaging modality.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI's ruff 0.16 caught six lint errors the pinned pre-commit ruff 0.11
didn't. They're all cosmetic / lint-shape fixes with no behaviour change:
* D205 x2 — docstring summary now sits on its own line, followed by a
blank line before the description block.
* W293 — trailing whitespace on a blank line (auto-fixed by ruff).
* PLW2901 — silence "loop variable rebinding" via `# noqa` on the
`v = preferred` re-target (kept because renaming would ripple).
* PERF102 — `for _key, t2w in dict.items()` → `for t2w in dict.values()`
(the key was never used).
* TRY300 — move successful `return` from the `try` body into an
`else:` block after the except.
Unblocks the ruff linter GitHub Action on this PR.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…x into dicom-extract-cleanup
The AttributeError branch already handled non-imaging DICOMs that lack `ImagePositionPatient`. Callers that hand `get_plane_dicom` an empty list (e.g. the new `dicom_renamer` when there are no DICOMs available), a dict, or a stub object also mean "no plane to compute" but tripped IndexError / KeyError / TypeError inside `create_affine`, which then surfaced as a warning and bogus `acq-None` filenames. Add IndexError / KeyError / TypeError to the silent-catch tuple so "no plane" cases stay quiet while genuine plane-detection bugs still warn via the general Exception branch below. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New module `TPTBox/core/dicom/dicom_renamer.py` walks an already-extracted
dataset root, feeds each sidecar JSON back through the current
`extract_keys_from_json` + BIDS path logic, and moves each file family to
its new path. Two use cases:
1. Sanitise BIDS-illegal subject ids. BIDS forbids `_` inside an entity
value (it's the entity separator). Datasets imported before that
rule was enforced end up with names like `sub-180217_711279`,
`sub-1YI_Za7zmmM`, `sub--1c0TOvqxPw` — legal-ish on disk but
`BIDS_FILE` parses them wrong. Default mode strips `_` and any
leading `-`.
2. Compact numeric ids. Passing `subject_prefix="ID"` reassigns every
subject to `ID001`, `ID002`, … zero-padded to
`subject_number_width` (default 3). The old → new mapping is
written to `<dataset_root>/<info_dir>/subject_map.tsv` so the
original ids stay recoverable.
Additional benefit: any change to `extract_keys_from_json` upstream
(view/laterality/BodyPart extraction, modality fallback, ordering fixes
in the series-description map) becomes effective on-disk without a
full DICOM re-extraction — the JSON sidecar already carries the same
metadata dict the DICOM did.
Performance shortcut: the plane is read from the sidecar's `grid` block
(populated by `_add_grid_info_to_json` during extraction) rather than
re-loaded from the NIfTI. A small `_FakeDicomList` stub keeps
`extract_keys_from_json` from touching disk at all — dry-runs cost O(N
JSONs) file opens instead of loading every NIfTI. Layout composition is
done manually (not via `_generate_bids_path`) so flat datasets like
`TOF_MPRAGE/sub-*/…` don't lose the subject folder to the parent-slot
default.
Ships with a `python -m TPTBox.core.dicom.dicom_renamer …` CLI: dry-run
by default, `--apply` to actually move files, `--parent ''` for flat
datasets, `--subject-prefix ID` for the numeric rename.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Analysis of the TOF-MPRAGE dataset shows 851 of 1248 files currently labelled ``_mr`` are already classified correctly by the current map on a re-run. Of the remaining 397, three tight patterns cover 324 (~81 %): * ``.*3di[_-]?mc.*`` → ``TOF`` — Philips 3D-Inflow Motion-Corrected High-Resolution MIP projections (ImageType carries ``PROJECTION IMAGE``, MRAcquisitionType is ``3D``, ScanningSequence ``GR``). Covers 318 files, all variants of ``3DI_MC_HR cc`` / ``rl`` / ``s3DI_MC_HR REC``. * ``.*dyn.*post.*km.*`` → ``DCE`` — dynamic post-contrast MR series (``\d+ Dyn post KM``). Requires all three tokens so a bare ``dyn`` doesn't over-match generic dynamic acquisitions. * ``.*halsgef.*`` → ``angio`` — German ``Halsgefäße`` neck-vessel angiography. Placed before the greedy ``.*t2.*`` / ``.*t1.*`` / ``.*mp?ra?ge?.*`` catch-alls so the specific classification wins. Remainder (73 files) is genuinely ambiguous or corrupt SeriesDescription (dates, empty strings, internal abbreviations) and legitimately stays ``mr``. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two additions so a re-run after a partial or interrupted apply is safe and complete: 1. Reload the previous `subject_map.tsv` on start. Folders that already match a `new_sub` value from the prior run map to themselves; folders whose id is still a `key` in the map get the previously assigned new id back; only unseen subjects claim a fresh number, taken from the next slot beyond the highest already-used one. Same idempotence for the sanitise-in-place branch. 2. Second pass after the JSON-driven rename: sweep any file left behind in an old `sub-<OLD>/` tree (typical orphans are `.bval` / `.bvec` companions and DWI-derived `_dwi_ADC.nii.gz` maps). Each orphan looks for a twin in the new subject folder by `sequ-<N>` match — when found, it inherits the twin's folder + stem so BIDS stem-linkage stays intact (`_dwi.bval` next to `_DWI.nii.gz` becomes `_DWI.bval`). Files that can't be twinned fall back to the session level of the new subject folder with just the subject prefix swapped. Users can now re-run the same command safely and pick up companion files that the first pass missed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…x into dicom-extract-cleanup
5 tasks
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.
Re-opens the work from #152 after renaming the head branch (
Point-Reg→dicom-extract-cleanup). Content identical; description below is the same as before.Summary
Cleanup pass on
TPTBox/core/dicom/. Splits into 10 focused commits (plus the pre-existing07aac40 fix multi echo bug). Every commit is reviewable on its own.Correctness fixes
88ab7e8—_extract_nii_from_dicomreturnsFalseon unknown exceptions, deletes the half-written sidecar, and logs. Previously the bareexcept Exceptionfell through toreturn True, so_add_grid_info_to_json/_split_multi_echo_dixonthen ran on a NIfTI that was never written.391a28e—_classic_get_grouped_dicomsno longer producesNaNdirections on multi-echo series. New helper_split_by_echo_numbersruns stack detection on a single-echo subset, then re-attaches remaining echoes byImagePositionPatient.f0c70f4—_split_multi_echo_dixontriggers on any 4-D output (not justpart in ("magnitude", "phase")). Also switched frame/TE iteration tozip_strict.0894e46—_find_all_filesyieldsPathinstances one at a time instead of yielding the rawlist[Path]first, which downstream code cast tostrand broke on.7f9e38b—_inc_keycaps its retry loop at 10 000 iterations and raisesRuntimeErrorwith the offending key instead of spinning forever on a brokenpath_existscallback.a09de6a—get_plane_dicom's real call now uses0.8(matches the docstring default); the bare-except now logs a warning so downstreamacq-Nonefiles are traceable.Perf / usability
4c63431—_read_dicom_filesgates on a_looks_like_dicomcheck (extension blocklist +DICMmagic at offset 128) beforepydicom.dcmread. Skips reports, thumbnails, JSON sidecars, etc. on mixed source trees.7a7ade2— Docs:n_cpubehaviour spelled out (1 sequential / >1 N threads / None →ThreadPoolExecutor-Default ≈min(32, cpu_count + 4)).Features
e1cddbc— Expandedmap_series_description_to_file_format_default:.*t2.*/.*t1.*catch-alls that were misclassifying them.ME_vibe_fatquant_*,mevibe,fatquant,fatfrac,pdff, GEideal).survey,refscan,smartexam).5280eb8— Modality fallback + BIDS format whitelist + view/laterality/bodypart:bids_constants.formatsgrows byxray,us,nm,sc,photo,endoscopy,rtimage,rtstruct,rtdose,rtplan,ot.extract_keys_from_jsonno longer raisesNotImplementedErrorfor CR/DX/RG/PX/IO/MG/US/NM/SC/OP/XC/ES/RT*/OT — sensible defaults per DICOM modality group, with a warning + tag-based fallback for unknowns._apply_view_keysand_apply_bodypart_keyliftViewPosition,Laterality/ImageLaterality, andBodyPartExaminedintoacq/part/desc, so MG four-view screenings, bilateral radiographs, ophthalmic photos, and multi-region NM/US sessions no longer collapse onto identical filenames.Test plan
pytest unit_tests/— 595 passed, 5 skipped (unchanged from base).acq-ax(notacq-iso) and the classic stack detector no longer over-splits it.🤖 Generated with Claude Code