From e077b5fc79efe2cc3adb3deaa1c850ea997fd196 Mon Sep 17 00:00:00 2001 From: Jules GRIVOT PELISSON Date: Tue, 27 Oct 2026 09:33:11 -0400 Subject: [PATCH 1/2] FIX: name each AREG IOS-CBCT patient folder as its files do and hand back the oriented CBCT beside the meshes --- .../src/sadt_areg_ioscbct/__init__.py | 5 +- .../src/sadt_areg_ioscbct/dispatch.py | 47 +++++++++++++-- .../src/sadt_areg_ioscbct/pipeline.py | 51 +++++++++++++++- tools/AREG/AREG_IOSCBCT/tests/test_pairing.py | 36 ++++++++++- .../AREG/AREG_IOSCBCT/tests/test_register.py | 60 +++++++++---------- tools/AREG/AREG_IOSCBCT/tests/test_run.py | 6 +- tools/AREG/AREG_IOSCBCT/tests/test_tools.py | 40 ++++++++++++- 7 files changed, 199 insertions(+), 46 deletions(-) diff --git a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py index bd94bfb..a6e18ef 100644 --- a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py +++ b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py @@ -80,7 +80,10 @@ def run( digits of the patient identifier, the two modalities being named by different conventions. output_dir: Where the registered meshes, their 4x4 matrices and - `AREG_report.json` are written. Nothing is written outside it. + `AREG_report.json` are written, one folder per patient named as + its files name it (`case_042`). When the CBCT was oriented first, + the oriented scan the meshes are registered onto and its `.tfm` + are written beside them. Nothing is written outside it. automation: Registration takes both landmark sets and predicts nothing -- no other tool is called and no GPU is needed. Semi-Automated labels the crowns and predicts both sets. Fully-Automated orients the CBCT diff --git a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/dispatch.py b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/dispatch.py index a72f2cf..2785279 100644 --- a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/dispatch.py +++ b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/dispatch.py @@ -21,7 +21,7 @@ import numpy as np -from sadt_areg_common import catalogs +from sadt_areg_common import catalogs, pairing from sadt_areg_common.errors import ToolInputError from . import geometry, pipeline, progress, tools, where @@ -294,9 +294,36 @@ def _spans(automation: str, predict_ios: bool, predict_cbct: bool) -> dict: return spans +def _carry_cbct(cbct_path: str, folder: str, output_dir: str) -> dict: + """Copy the CBCT the meshes were registered onto into the patient's folder. + + In the Fully-Automated mode that is ASO's oriented scan, not the caller's: + every arch is written in its frame, and it lived only in the job's work + directory. Opened over the original scan, the meshes were off by the whole + rotation ASO applied, with nothing in the results to put them back on. Its + transform (original scan to oriented one, ASO's `_transform.tfm`) + comes too, for a reader who wants the meshes on the original scan instead. + + Returns the report fields naming what was copied, relative to `output_dir`. + """ + os.makedirs(folder, exist_ok=True) + copied = {} + stem = pairing.split_scan_extension(os.path.basename(cbct_path))[0] + transform = os.path.join(os.path.dirname(cbct_path), f"{stem}_transform.tfm") + for field, source in (("oriented_cbct", cbct_path), ("cbct_transform", transform)): + if not os.path.isfile(source): + logger.warning("No %s to copy beside the registered meshes", field.replace("_", " ")) + continue + destination = os.path.join(folder, os.path.basename(source)) + shutil.copy2(source, destination) + copied[field] = os.path.relpath(destination, output_dir) + return copied + + def register(ios_dir: str, cbct_dir: str, ios_landmark_dir: str, cbct_landmark_dir: str, output_dir: str, suffix: str, report: dict, max_dist: float, - progress_span: tuple = (0.0, 1.0), produced_by: dict = None) -> None: + progress_span: tuple = (0.0, 1.0), produced_by: dict = None, + carry_cbct: bool = False) -> None: """The registration proper, once every landmark exists. `progress_span` is the slice of the run's progress bar this phase fills. @@ -311,6 +338,9 @@ def register(ios_dir: str, cbct_dir: str, ios_landmark_dir: str, cbct_landmark_d source the message names, and its class -- an empty folder the caller sent is theirs to fix (ToolInputError), one a tool returned is a fault on this side (RuntimeError). + + `carry_cbct` copies each patient's CBCT, and the transform beside it, into + that patient's folder: see `_carry_cbct`. """ produced_by = produced_by or {} @@ -357,6 +387,7 @@ def error_for(*roles): progress.report(index, len(paired), "patient", start=progress_span[0], end=progress_span[1]) position = f"patient {index}/{len(paired)}" + folder = os.path.join(output_dir, data["name"]) entry = {"cbct": os.path.basename(data["cbct"]), "meshes": {}} # Narrowed to this patient BEFORE the jaw is looked at: see _for_patient. own_ios = _for_patient(ios_landmarks, patient, sole_patient) @@ -386,7 +417,7 @@ def error_for(*roles): mesh, moving, fixed, cbct_surface, on_enamel, max_dist=max_dist ) destination = os.path.join( - output_dir, patient, f"{os.path.splitext(name)[0]}_{suffix}.vtk" + folder, f"{os.path.splitext(name)[0]}_{suffix}.vtk" ) _write_mesh(mesh.transform(matrix, inplace=False), destination) # splitext, not `destination.replace(".vtk", ...)`: str.replace @@ -415,7 +446,11 @@ def error_for(*roles): statuses = [m["status"] for m in entry["meshes"].values()] entry["status"] = ("ok" if "ok" in statuses else "review" if "review" in statuses else "failed") - report["patients"][patient] = entry + if carry_cbct and entry["status"] != "failed": + entry.update(_carry_cbct(data["cbct"], folder, output_dir)) + # Under the name the files carry, never the matching key: see + # pipeline.patient_name. + report["patients"][data["name"]] = entry report["meshes_registered"] = total_meshes - len(failures) report["meshes_failed"] = len(failures) @@ -596,6 +631,10 @@ def main(ios, cbct, output_dir, automation=None, ios_landmarks=None, cbct_landma # Registration mode, which makes none of them. progress_span=spans["register"], produced_by=produced_by, + # The meshes are registered onto the CBCT ASO oriented, a volume + # the caller never had: without it beside them, they sit nowhere + # near the scan the caller opens them over. + carry_cbct=produced_by.get("cbct") == "ASO", ) finally: shutil.rmtree(work_dir, ignore_errors=True) diff --git a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/pipeline.py b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/pipeline.py index 558fc6f..2e3f399 100644 --- a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/pipeline.py +++ b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/pipeline.py @@ -13,6 +13,7 @@ import json import logging import os +import re import numpy as np @@ -53,10 +54,37 @@ def patient_key(filename: str) -> str: return digits.lstrip("0") or digits or stem +_WORD = re.compile(r"[A-Za-z0-9]+") + + +def patient_name(filename: str) -> str: + """The patient as the file's own name spells it: `case_042`, not `42`. + + `patient_key` is what MATCHES the two modalities, and it has to be crude to + do it; it is no name to show anyone. A folder called `42` beside a scan + called `case_042_T1_CBCT.nii.gz` broke the one thing a reader relies on to + find a patient again. This keeps the name up to the end of its first word + holding a digit -- the patient's number -- skipping a timepoint word, so + `case_042_T1_L.stl` gives `case_042` and `P_0001_T2.nii.gz` gives `P_0001`. + A name with no digit at all is kept whole. + """ + stem = pairing.split_scan_extension(os.path.basename(filename))[0] + for word in _WORD.finditer(stem): + if word.group().lower() in ("t0", "t1", "t2"): + continue + if any(c.isdigit() for c in word.group()): + return stem[: word.end()] + return stem + + def discover(ios_dir: str, cbct_dir: str, ios_source: str = "'ios'", cbct_source: str = "'cbct'", ios_error=ToolInputError, cbct_error=ToolInputError) -> tuple: - """`({patient: {"ios": [paths], "cbct": path}}, {patient: why})`. + """`({key: {"ios": [paths], "cbct": path, "name": name}}, {name: why})`. + + Keyed by `patient_key`, which is what the landmark files are matched on; + `name` is `patient_name` of the patient's first intraoral mesh -- the files + the output folder holds -- or of its CBCT when it has none. `ios_source` and `cbct_source` name where each folder came from in the refusal -- the caller's argument, or the tool that produced it -- and @@ -96,10 +124,12 @@ def discover(ios_dir: str, cbct_dir: str, ios_source: str = "'ios'", paired, unpaired = {}, {} for key in sorted(set(ios) | set(cbct)): + name = patient_name(min(ios[key]) if key in ios else cbct[key]) if key in ios and key in cbct: - paired[key] = {"ios": sorted(ios[key]), "cbct": cbct[key]} + paired[key] = {"ios": sorted(ios[key]), "cbct": cbct[key], "name": name} else: - unpaired[key] = "no CBCT" if key in ios else "no intraoral scan" + unpaired[name] = "no CBCT" if key in ios else "no intraoral scan" + _distinct_names(paired) if not paired: # Counts, never the keys: a key is built from the caller's file names, @@ -123,6 +153,21 @@ def discover(ios_dir: str, cbct_dir: str, ios_source: str = "'ios'", return paired, unpaired +def _distinct_names(paired: dict) -> None: + """Two patients never share a folder. + + The key reads every digit of a name and the name stops at the first word + holding one, so `case_042_scan2` and `case_042_scan3` are two keys and one + name. The second then takes its key too, rather than writing its arches + over the first patient's. + """ + seen = set() + for key, data in paired.items(): + if data["name"] in seen: + data["name"] = f"{data['name']}_{key}" + seen.add(data["name"]) + + def read_landmarks(path: str) -> dict: """`{label: [x, y, z]}` from a Slicer markups file or a plain JSON one. diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_pairing.py b/tools/AREG/AREG_IOSCBCT/tests/test_pairing.py index 24dd484..315a7f0 100644 --- a/tools/AREG/AREG_IOSCBCT/tests/test_pairing.py +++ b/tools/AREG/AREG_IOSCBCT/tests/test_pairing.py @@ -83,6 +83,38 @@ def test_the_compound_extension_is_split_as_one_unit(): assert pipeline.patient_key("P7.nii.gz") == pipeline.patient_key("P7.vtk") == "7" +# --------------------------------------------------------------------------- +# patient_name +# --------------------------------------------------------------------------- + +def test_the_patient_name_keeps_the_files_own_spelling(): + """A folder called `42` beside `case_042_...` files was no name to find a + patient by: the name keeps its prefix and its zeros.""" + assert pipeline.patient_name("case_042_T1_L.stl") == "case_042" + assert pipeline.patient_name("case_042_T1_CBCT.nii.gz") == "case_042" + assert pipeline.patient_name("P_0001_T2.nii.gz") == "P_0001" + assert pipeline.patient_name("P001_T2_U_Seg.vtk") == "P001" + + +def test_a_leading_timepoint_is_not_taken_for_the_patient(): + assert pipeline.patient_name("T1_case_042_U.vtk") == "T1_case_042" + + +def test_a_name_with_no_digit_is_kept_whole(): + assert pipeline.patient_name("upper_scan.vtk") == "upper_scan" + + +def test_two_patients_never_share_a_folder(tmp_path): + """Two keys, one name: the second keeps its key so it cannot overwrite.""" + write_mesh(tmp_path / "ios" / "case_042_scan2_U.vtk") + write_mesh(tmp_path / "ios" / "case_042_scan3_U.vtk") + write_volume(tmp_path / "cbct" / "case_042_scan2.nii.gz") + write_volume(tmp_path / "cbct" / "case_042_scan3.nii.gz") + + paired, _unpaired = pipeline.discover(str(tmp_path / "ios"), str(tmp_path / "cbct")) + assert sorted(data["name"] for data in paired.values()) == ["case_042", "case_042_423"] + + # --------------------------------------------------------------------------- # discover # --------------------------------------------------------------------------- @@ -92,7 +124,7 @@ def test_a_patient_present_in_both_modalities_is_paired(tmp_path): volume = write_volume(tmp_path / "cbct" / "P_0001_T2.nii.gz") paired, unpaired = pipeline.discover(str(tmp_path / "ios"), str(tmp_path / "cbct")) - assert paired == {"1": {"ios": [mesh], "cbct": volume}} + assert paired == {"1": {"ios": [mesh], "cbct": volume, "name": "P001"}} assert unpaired == {} @@ -119,7 +151,7 @@ def test_a_patient_with_only_one_modality_is_reported_not_dropped(tmp_path): paired, unpaired = pipeline.discover(str(tmp_path / "ios"), str(tmp_path / "cbct")) assert set(paired) == {"1"} - assert unpaired == {"2": "no CBCT", "3": "no intraoral scan"} + assert unpaired == {"P002": "no CBCT", "P_0003": "no intraoral scan"} def test_no_pair_at_all_gives_both_counts_and_no_key(tmp_path): diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_register.py b/tools/AREG/AREG_IOSCBCT/tests/test_register.py index 34e9874..da2a9ac 100644 --- a/tools/AREG/AREG_IOSCBCT/tests/test_register.py +++ b/tools/AREG/AREG_IOSCBCT/tests/test_register.py @@ -30,8 +30,8 @@ def test_a_run_writes_the_mesh_its_matrix_and_the_report(tmp_path): run_registration(tmp_path) assert tree_of(tmp_path / "out") == sorted([ "AREG_report.json", - os.path.join("1", "P001_T2_U_Reg.vtk"), - os.path.join("1", "P001_T2_U_Reg_matrix.npy"), + os.path.join("P001", "P001_T2_U_Reg.vtk"), + os.path.join("P001", "P001_T2_U_Reg_matrix.npy"), ]) @@ -43,8 +43,8 @@ def test_the_written_mesh_is_the_input_moved_by_the_written_matrix(tmp_path): run_registration(tmp_path) before = read_mesh_points(tmp_path / "ios" / "P001_T2_U.vtk") - after = read_mesh_points(tmp_path / "out" / "1" / "P001_T2_U_Reg.vtk") - matrix = np.load(str(tmp_path / "out" / "1" / "P001_T2_U_Reg_matrix.npy")) + after = read_mesh_points(tmp_path / "out" / "P001" / "P001_T2_U_Reg.vtk") + matrix = np.load(str(tmp_path / "out" / "P001" / "P001_T2_U_Reg_matrix.npy")) assert geometry.apply(before, matrix) == pytest.approx(after, abs=1e-4) @@ -55,7 +55,7 @@ def test_the_matrix_is_the_one_the_landmarks_asked_for(tmp_path): cohort(tmp_path) run_registration(tmp_path) - matrix = np.load(str(tmp_path / "out" / "1" / "P001_T2_U_Reg_matrix.npy")) + matrix = np.load(str(tmp_path / "out" / "P001" / "P001_T2_U_Reg_matrix.npy")) assert matrix[:3, :3] == pytest.approx(np.eye(3), abs=1e-6) assert matrix[:3, 3] == pytest.approx([3.0, -2.0, 1.0], abs=1e-6) @@ -65,10 +65,10 @@ def test_each_patient_gets_a_folder_of_its_own(tmp_path): run_registration(tmp_path) assert tree_of(tmp_path / "out") == sorted([ "AREG_report.json", - os.path.join("1", "P001_T2_U_Reg.vtk"), - os.path.join("1", "P001_T2_U_Reg_matrix.npy"), - os.path.join("2", "P002_T2_U_Reg.vtk"), - os.path.join("2", "P002_T2_U_Reg_matrix.npy"), + os.path.join("P001", "P001_T2_U_Reg.vtk"), + os.path.join("P001", "P001_T2_U_Reg_matrix.npy"), + os.path.join("P002", "P002_T2_U_Reg.vtk"), + os.path.join("P002", "P002_T2_U_Reg_matrix.npy"), ]) @@ -78,7 +78,7 @@ def test_both_arches_of_one_patient_are_registered_separately(tmp_path): run_registration(tmp_path) report = read_report(tmp_path / "out") - assert set(report["patients"]["1"]["meshes"]) == {"P001_T2_U.vtk", "P001_T2_L.vtk"} + assert set(report["patients"]["P001"]["meshes"]) == {"P001_T2_U.vtk", "P001_T2_L.vtk"} def test_the_suffix_reaches_every_name_it_writes(tmp_path): @@ -86,8 +86,8 @@ def test_the_suffix_reaches_every_name_it_writes(tmp_path): run_registration(tmp_path, output_suffix="Study7") assert tree_of(tmp_path / "out") == sorted([ "AREG_report.json", - os.path.join("1", "P001_T2_U_Study7.vtk"), - os.path.join("1", "P001_T2_U_Study7_matrix.npy"), + os.path.join("P001", "P001_T2_U_Study7.vtk"), + os.path.join("P001", "P001_T2_U_Study7_matrix.npy"), ]) @@ -96,7 +96,7 @@ def test_a_blank_suffix_falls_back_to_reg(tmp_path, given): cohort(tmp_path) run_registration(tmp_path, output_suffix=given) assert read_report(tmp_path / "out")["output_suffix"] == "Reg" - assert os.path.join("1", "P001_T2_U_Reg.vtk") in tree_of(tmp_path / "out") + assert os.path.join("P001", "P001_T2_U_Reg.vtk") in tree_of(tmp_path / "out") @pytest.mark.parametrize("given", ["../escape", "a/b", os.sep + "abs"]) @@ -117,7 +117,7 @@ def test_a_mesh_whose_own_stem_carries_vtk_still_names_its_matrix_correctly(tmp_ str(tmp_path / "ios" / "P001_T2_U.vtk"), str(tmp_path / "ios" / "P001_T2_U.vtk.vtk") ) run_registration(tmp_path) - assert os.path.join("1", "P001_T2_U.vtk_Reg_matrix.npy") in tree_of(tmp_path / "out") + assert os.path.join("P001", "P001_T2_U.vtk_Reg_matrix.npy") in tree_of(tmp_path / "out") def test_the_output_directory_is_created_when_it_does_not_exist(tmp_path): @@ -184,14 +184,14 @@ def test_the_report_says_what_ran_and_on_what(tmp_path): assert report["output_suffix"] == "Reg" assert isinstance(report["duration_seconds"], float) - patient = report["patients"]["1"] + patient = report["patients"]["P001"] assert patient["status"] == "ok" assert patient["cbct"] == "P_0001_T2.nii.gz" mesh = patient["meshes"]["P001_T2_U.vtk"] assert mesh["status"] == "ok" assert mesh["landmarks_used"] == sorted(UPPER_LABELS) assert mesh["landmarks_dropped"] == [] - assert mesh["output"] == os.path.join("1", "P001_T2_U_Reg.vtk") + assert mesh["output"] == os.path.join("P001", "P001_T2_U_Reg.vtk") def test_the_report_carries_no_server_side_path(tmp_path): @@ -211,7 +211,7 @@ def test_a_landmark_only_one_side_names_is_reported_as_dropped(tmp_path): write_markups(tmp_path / "cbct_lm" / "P_0001_T2_lm_Pred.mrk.json", extended) run_registration(tmp_path) - mesh = read_report(tmp_path / "out")["patients"]["1"]["meshes"]["P001_T2_U.vtk"] + mesh = read_report(tmp_path / "out")["patients"]["P001"]["meshes"]["P001_T2_U.vtk"] assert mesh["landmarks_dropped"] == ["EXTRA"] @@ -220,7 +220,7 @@ def test_a_patient_with_only_one_modality_is_named_in_the_report(tmp_path): write_mesh(tmp_path / "ios" / "P009_T2_U.vtk") run_registration(tmp_path) - assert read_report(tmp_path / "out")["unpaired"] == {"9": "no CBCT"} + assert read_report(tmp_path / "out")["unpaired"] == {"P009": "no CBCT"} def test_the_icp_refines_on_the_cbct_the_run_was_given(tmp_path): @@ -242,7 +242,7 @@ def test_the_icp_refines_on_the_cbct_the_run_was_given(tmp_path): box=((3.0, -2.0, 1.0), (23.0, 12.0, 11.0))) run_registration(tmp_path) - icp = read_report(tmp_path / "out")["patients"]["1"]["meshes"][ + icp = read_report(tmp_path / "out")["patients"]["P001"]["meshes"][ "P001_T2_U.vtk"]["icp"] assert icp is not None assert icp["pairs"] >= 3 and icp["fitness"] > 0 @@ -260,7 +260,7 @@ def test_a_cbct_no_reader_can_open_falls_back_to_the_landmarks_and_says_so(tmp_p cohort(tmp_path) run_registration(tmp_path) - patient = read_report(tmp_path / "out")["patients"]["1"] + patient = read_report(tmp_path / "out")["patients"]["P001"] assert patient["meshes"]["P001_T2_U.vtk"]["icp"] is None assert "could not be contoured" in patient["cbct_surface_error"] assert str(tmp_path) not in patient["cbct_surface_error"] @@ -283,11 +283,11 @@ def test_one_mesh_failing_does_not_cost_the_batch(tmp_path): run_registration(tmp_path) report = read_report(tmp_path / "out") - assert report["patients"]["1"]["status"] == "ok" - failed = report["patients"]["2"] + assert report["patients"]["P001"]["status"] == "ok" + failed = report["patients"]["P002"] assert failed["status"] == "failed" assert "share only 1 landmark(s)" in failed["meshes"]["P002_T2_U.vtk"]["error"] - assert os.path.join("1", "P001_T2_U_Reg.vtk") in tree_of(tmp_path / "out") + assert os.path.join("P001", "P001_T2_U_Reg.vtk") in tree_of(tmp_path / "out") def test_a_batch_where_nothing_registered_raises_with_the_cause_in_it(tmp_path): @@ -328,7 +328,7 @@ def test_a_mesh_with_no_landmarks_of_its_own_names_the_patient_and_the_side(tmp_ os.remove(str(tmp_path / "ios_lm" / "P002_T2_U_lm_Pred.mrk.json")) run_registration(tmp_path) - failed = read_report(tmp_path / "out")["patients"]["2"]["meshes"]["P002_T2_U.vtk"] + failed = read_report(tmp_path / "out")["patients"]["P002"]["meshes"]["P002_T2_U.vtk"] assert failed["status"] == "failed" assert "this patient" in failed["error"] assert "the intraoral" in failed["error"] @@ -355,8 +355,8 @@ def test_an_unreadable_landmark_file_is_skipped_with_its_position(tmp_path, capl with caplog.at_level(logging.INFO, logger="sadt_areg_ioscbct"): run_registration(tmp_path) report = read_report(tmp_path / "out") - assert report["patients"]["1"]["status"] == "ok" - assert report["patients"]["2"]["status"] == "failed" + assert report["patients"]["P001"]["status"] == "ok" + assert report["patients"]["P002"]["status"] == "failed" unreadable = [r.getMessage() for r in caplog.records if "landmark file 2 of 2" in r.getMessage()] assert len(unreadable) == 1 assert unreadable[0].startswith( @@ -391,7 +391,7 @@ def test_each_patient_is_registered_on_its_own_landmarks(tmp_path): ) run_registration(tmp_path) - second = np.load(str(tmp_path / "out" / "2" / "P002_T2_U_Reg_matrix.npy")) + second = np.load(str(tmp_path / "out" / "P002" / "P002_T2_U_Reg_matrix.npy")) assert second[:3, 3] == pytest.approx([100.0, 100.0, 100.0], abs=1e-5) @@ -410,8 +410,8 @@ def test_a_patients_two_arches_do_not_borrow_each_others_points(tmp_path): os.remove(str(tmp_path / "cbct_lm" / "P_0001_T2_lm_Pred.mrk.json")) run_registration(tmp_path) - upper = np.load(str(tmp_path / "out" / "1" / "P001_T2_U_Reg_matrix.npy")) - lower = np.load(str(tmp_path / "out" / "1" / "P001_T2_L_Reg_matrix.npy")) + upper = np.load(str(tmp_path / "out" / "P001" / "P001_T2_U_Reg_matrix.npy")) + lower = np.load(str(tmp_path / "out" / "P001" / "P001_T2_L_Reg_matrix.npy")) assert upper[:3, 3] == pytest.approx([3.0, -2.0, 1.0], abs=1e-5) assert lower[:3, 3] == pytest.approx([50.0, 0.0, 0.0], abs=1e-5) @@ -427,7 +427,7 @@ def test_one_cbct_landmark_file_serves_both_arches(tmp_path): ) run_registration(tmp_path) - report = read_report(tmp_path / "out")["patients"]["1"]["meshes"] + report = read_report(tmp_path / "out")["patients"]["P001"]["meshes"] assert report["P001_T2_U.vtk"]["landmarks_used"] == sorted(UPPER_LABELS) assert report["P001_T2_L.vtk"]["landmarks_used"] == sorted(LOWER_LABELS) diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_run.py b/tools/AREG/AREG_IOSCBCT/tests/test_run.py index b28bfb5..8b565a8 100644 --- a/tools/AREG/AREG_IOSCBCT/tests/test_run.py +++ b/tools/AREG/AREG_IOSCBCT/tests/test_run.py @@ -586,7 +586,7 @@ def drifted(*args, **kwargs): run_registration(tmp_path) report = json.loads((tmp_path / "out" / "AREG_report.json").read_text()) - patient = report["patients"]["1"] + patient = report["patients"]["P001"] assert patient["status"] == "review" for mesh in patient["meshes"].values(): assert mesh["status"] == "review" @@ -603,6 +603,6 @@ def test_an_arch_close_to_its_landmarks_stays_ok(tmp_path): run_registration(tmp_path) report = json.loads((tmp_path / "out" / "AREG_report.json").read_text()) - assert report["patients"]["1"]["status"] == "ok" + assert report["patients"]["P001"]["status"] == "ok" assert report["meshes_to_review"] == 0 - assert all("review" not in mesh for mesh in report["patients"]["1"]["meshes"].values()) + assert all("review" not in mesh for mesh in report["patients"]["P001"]["meshes"].values()) diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_tools.py b/tools/AREG/AREG_IOSCBCT/tests/test_tools.py index cf95d63..73893d0 100644 --- a/tools/AREG/AREG_IOSCBCT/tests/test_tools.py +++ b/tools/AREG/AREG_IOSCBCT/tests/test_tools.py @@ -17,7 +17,9 @@ import pytest -from conftest import FakeSup, cohort, read_report, write_markups, write_mesh, UPPER_LABELS, moved +from conftest import ( + FakeSup, cohort, read_report, write_markups, write_mesh, write_volume, UPPER_LABELS, moved, +) from sadt_areg_ioscbct import dispatch, tools from sadt_areg_common import catalogs from sadt_areg_common.errors import SupervisorRequired, ToolInputError @@ -269,7 +271,7 @@ def test_semi_automated_labels_the_crowns_and_predicts_both_landmark_sets(tmp_pa dispatch.main(**arguments) assert [name for name, _params in sup.calls] == ["Crown_Seg", "ALI_IOS", "ALI_CBCT"] - assert read_report(tmp_path / "out")["patients"]["1"]["status"] == "ok" + assert read_report(tmp_path / "out")["patients"]["P001"]["status"] == "ok" def test_semi_automated_does_not_orient_anything(tmp_path): @@ -327,6 +329,38 @@ def test_fully_automated_asks_aso_for_its_frame_and_no_bundle(tmp_path): assert "reference" not in params +def test_fully_automated_hands_back_the_cbct_the_meshes_were_registered_onto(tmp_path): + """The meshes are in the frame of ASO's oriented scan. Without it beside + them they were opened over the original CBCT, off by ASO's whole rotation, + and the only copy had gone with the job's work directory.""" + oriented = tmp_path / "oriented" + write_volume(oriented / "P_0001_T2_Or.nii.gz") + (oriented / "P_0001_T2_Or_transform.tfm").write_text("#Insight Transform File V1.0\n") + _sup, arguments = semi_automated( + tmp_path, + automation=catalogs.AUTOMATION_FULLY, + outputs={"ASO": lambda params: oriented}, + ) + dispatch.main(**arguments) + + folder = tmp_path / "out" / "P001" + assert (folder / "P_0001_T2_Or.nii.gz").is_file() + assert (folder / "P_0001_T2_Or_transform.tfm").is_file() + patient = read_report(tmp_path / "out")["patients"]["P001"] + assert patient["oriented_cbct"] == os.path.join("P001", "P_0001_T2_Or.nii.gz") + assert patient["cbct_transform"] == os.path.join("P001", "P_0001_T2_Or_transform.tfm") + + +def test_a_cbct_the_caller_sent_is_not_sent_back(tmp_path): + """Semi-Automated registers onto the caller's own scan: copying it back + would only double the archive.""" + _sup, arguments = semi_automated(tmp_path) + dispatch.main(**arguments) + + assert not (tmp_path / "out" / "P001" / "P_0001_T2.nii.gz").exists() + assert "oriented_cbct" not in read_report(tmp_path / "out")["patients"]["P001"] + + def test_a_sibling_tool_failing_takes_the_run_with_it(tmp_path): """Nothing downstream can be salvaged: there are no landmarks to register on, and inventing some would be a confident wrong answer.""" @@ -374,7 +408,7 @@ def test_the_predicted_landmarks_really_reach_the_registration(tmp_path): import numpy as np - matrix = np.load(str(tmp_path / "out" / "1" / "P001_T2_U_Reg_matrix.npy")) + matrix = np.load(str(tmp_path / "out" / "P001" / "P001_T2_U_Reg_matrix.npy")) assert matrix[0, 3] == pytest.approx(7.0, abs=1e-5) From 34d153e9de924f1ddb07e6d59ff5628a7726b551 Mon Sep 17 00:00:00 2001 From: Jules GRIVOT PELISSON Date: Tue, 27 Oct 2026 11:28:51 -0400 Subject: [PATCH 2/2] ADD: let AREG read a cohort kept as one folder per patient, chosen beside the per-type layout --- scripts/describe.py | 34 ++- .../AREG_CBCT/src/sadt_areg_cbct/__init__.py | 75 +++++-- .../AREG_CBCT/src/sadt_areg_cbct/layout.py | 29 ++- .../AREG_CBCT/tests/test_patient_folders.py | 56 +++++ .../AREG_IOS/src/sadt_areg_ios/__init__.py | 84 ++++++-- .../AREG/AREG_IOS/src/sadt_areg_ios/layout.py | 29 ++- .../AREG_IOS/tests/test_patient_folders.py | 56 +++++ .../src/sadt_areg_ioscbct/__init__.py | 86 +++++++- .../src/sadt_areg_ioscbct/layout.py | 29 ++- .../tests/test_patient_folders.py | 73 +++++++ tools/AREG/AREG_IOSCBCT/tests/test_run.py | 9 +- .../common/src/sadt_areg_common/catalogs.py | 59 +++++ .../src/sadt_areg_common/patient_folders.py | 202 ++++++++++++++++++ .../AREG/common/tests/test_patient_folders.py | 156 ++++++++++++++ 14 files changed, 910 insertions(+), 67 deletions(-) create mode 100644 tools/AREG/AREG_CBCT/tests/test_patient_folders.py create mode 100644 tools/AREG/AREG_IOS/tests/test_patient_folders.py create mode 100644 tools/AREG/AREG_IOSCBCT/tests/test_patient_folders.py create mode 100644 tools/AREG/common/src/sadt_areg_common/patient_folders.py create mode 100644 tools/AREG/common/tests/test_patient_folders.py diff --git a/scripts/describe.py b/scripts/describe.py index 69ca9f3..78313f1 100644 --- a/scripts/describe.py +++ b/scripts/describe.py @@ -117,6 +117,13 @@ # an option the signature does not publish, which is what makes that hold. LAYOUT_KEYS = ("section", "ui", "groups", "visible_when", "options_when", "label", "hidden", + # {other argument: value(s)}: the argument is required whenever + # those hold. For an input that is needed in one layout of the + # request and absent from another -- AREG's T1 and T2, required + # with one folder per type and unused with one per patient -- so + # the panel neither marks it optional nor lets Apply go without + # it. The tool still refuses what is missing; this is the panel. + "required_when", # A vec2's two axes, low end first (or high end first to mirror # the axis). Declared here with the other layout keys because a # tool writes them in the same place, but they are NOT @@ -938,6 +945,7 @@ def layout_for(package, arguments): if name in arguments: _check_groups(where, hints, arguments[name]) _check_visible_when(where, hints, arguments) + _check_visible_when(where, hints, arguments, key="required_when") return declared @@ -966,17 +974,21 @@ def _check_groups(where, hints, argument): ) -def _check_visible_when(where, hints, arguments): - """A condition must name a real argument, and a value it can actually take.""" - conditions = hints.get("visible_when") +def _check_visible_when(where, hints, arguments, key="visible_when"): + """A condition must name a real argument, and a value it can actually take. + + `key` is the hint holding the condition: `visible_when`, or `required_when`, + which is written the same way and goes wrong the same way. + """ + conditions = hints.get(key) if conditions is None: return if not isinstance(conditions, dict): - raise SchemaError("{}: 'visible_when' must be a dict of argument -> value.".format(where)) + raise SchemaError("{}: '{}' must be a dict of argument -> value.".format(where, key)) for other, expected in conditions.items(): if other not in arguments: raise SchemaError( - "{}: 'visible_when' names '{}', which run() does not take.".format(where, other) + "{}: '{}' names '{}', which run() does not take.".format(where, key, other) ) offered = arguments[other].get("choices") if not offered: @@ -985,9 +997,9 @@ def _check_visible_when(where, hints, arguments): missing = [value for value in wanted if value not in offered] if missing: raise SchemaError( - "{}: 'visible_when' expects '{}' to be {}, which it never is. It " + "{}: '{}' expects '{}' to be {}, which it never is. It " "offers: {}.".format( - where, other, ", ".join(str(m) for m in missing), ", ".join(offered) + where, key, other, ", ".join(str(m) for m in missing), ", ".join(offered) ) ) @@ -1105,9 +1117,13 @@ def paired_for(module, arguments): spec = arguments.get(axis) if spec is None: raise SchemaError("{}: run() has no argument '{}'.".format(PAIRED_NAME, axis)) - if spec.get("type") != "path" or not spec.get("required"): + # A path, but not necessarily a required one: AREG's `t1`/`t2` are + # optional since the same cohort may arrive as one folder per patient + # instead, and a run in that layout fills neither. A client splits the + # axes a request fills. + if spec.get("type") != "path": raise SchemaError( - "{}: '{}' must be a required path argument to be split.".format(PAIRED_NAME, axis)) + "{}: '{}' must be a path argument to be split.".format(PAIRED_NAME, axis)) pairs = getattr(module, PAIRS_FUNCTION, None) if not inspect.isfunction(pairs): raise SchemaError( diff --git a/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/__init__.py b/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/__init__.py index 357df9c..8dd3d20 100644 --- a/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/__init__.py +++ b/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/__init__.py @@ -15,9 +15,12 @@ `tools.py`. `AREG_CBCT -> ASO -> ALI_CBCT` is the deepest chain in the family. """ +import shutil from pathlib import Path from typing import Literal +from sadt_areg_common import catalogs, pairing, patient_folders + from .dispatch import main @@ -54,10 +57,28 @@ def match(roots): # need to be scoped per mode. Until then the engine's own panel filters # nothing, and `pairing.is_scan_file` is what actually decides. +_ROLE = patient_folders.by_timepoint(pairing.is_scan_file, skip=catalogs.MASK_TOKENS) + + +def _filed_as(patient: str, role: str, relative: str, index: int) -> str: + """`_T1.nii.gz`: the name the pairing reads as that patient's T1.""" + name = relative.replace("\\", "/").rsplit("/", 1)[-1] + return f"{patient}_{role.upper()}{patient_folders.extension_of(name)}" + + def run( - t1: Path, - t2: Path, - output_dir: Path, + # First, because it decides which of the next three the panel asks for. + # Spelled out because `Literal` takes literals only; a test asserts it + # against `catalogs.LAYOUT_CHOICES`. + input_layout: Literal["One folder per type", "One folder per patient"] = "One folder per type", + # Not required any more, and not because they are optional: which of + # `t1`/`t2` or `patients` is needed depends on `input_layout`, and a field + # the panel marks required in the layout that does not read it would + # refuse to apply. `patient_folders.resolve` refuses what is missing. + t1: Path = "", + t2: Path = "", + patients: Path = "", + output_dir: Path = "", # "From the data" is the default and names no mode: `dispatch.derive_automation` # reads it off what the request supplies -- masks are Semi-Automated, none is # Fully-Automated, and the frame named in `orientation` below chooses the @@ -113,6 +134,12 @@ def run( """Register a follow-up CBCT onto its baseline, so the two can be compared. Args: + input_layout: How the files are organised: one folder for each type + of file (T1 and T2, or intraoral scans and CBCT) holding every + patient, or one folder for each patient holding all of that + patient's files. Rest the pointer on each option for an example. + patients: The folder of patient folders, in the one-folder-per-patient + layout. Each patient's folder name names its results. t1: The baseline scans -- one volume or a folder of them, searched recursively. A DICOM series is converted when `dicom_input` is set. t2: The follow-up scans, paired to T1 by patient key. @@ -165,21 +192,29 @@ def run( """ # itk-elastix and SimpleITK are imported inside the engine: CI imports this # module on every PR to publish the schema, and that must not cost them. - return main( - t1=t1, - t2=t2, - output_dir=output_dir, - automation=automation, - cbct_regions=regions, - t1_masks=t1_masks, - segmentations=segmentations, - segmentation_model=segmentation_model, - segmentation_label=segmentation_label, - cbct_reference=reference, - landmark_model=landmark_model, - dicom_input=dicom_input, - orientation=orientation, - output_suffix=output_suffix, - num_workers=num_workers, - sup=sup, + folders, scratch = patient_folders.resolve( + input_layout, patients, {"t1": t1, "t2": t2}, output_dir, _ROLE, _filed_as, ) + try: + result = main( + t1=folders["t1"], + t2=folders["t2"], + output_dir=output_dir, + automation=automation, + cbct_regions=regions, + t1_masks=t1_masks, + segmentations=segmentations, + segmentation_model=segmentation_model, + segmentation_label=segmentation_label, + cbct_reference=reference, + landmark_model=landmark_model, + dicom_input=dicom_input, + orientation=orientation, + output_suffix=output_suffix, + num_workers=num_workers, + sup=sup, + ) + finally: + if scratch: + shutil.rmtree(scratch, ignore_errors=True) + return result diff --git a/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/layout.py b/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/layout.py index 79810fc..d0bfed1 100644 --- a/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/layout.py +++ b/tools/AREG/AREG_CBCT/src/sadt_areg_cbct/layout.py @@ -14,6 +14,15 @@ from sadt_areg_common import catalogs _INPUTS = "Inputs" + +# Which of the two input shapes the panel asks for. The per-type fields and the +# folder of patients are never shown together: a reader who filled both would +# not know which one the run read. Each is REQUIRED in its own layout, which +# the signature cannot say -- both are optional there, since either may be +# the one not sent. +_BY_TYPE = {"input_layout": [catalogs.LAYOUT_BY_TYPE]} +_BY_PATIENT = {"input_layout": [catalogs.LAYOUT_BY_PATIENT]} + _REGISTRATION = "Registration" # Where the two inputs that SELECT a mode live, for the reason ASO puts its # landmark folder there: someone who already has masks, or who wants the T1 @@ -70,8 +79,24 @@ # end, and a ticked step pauses the run and opens what it produced in VISU. "stop_after": {"label": "Stop after"}, - "t1": {"section": _INPUTS, "label": "T1 (baseline)"}, - "t2": {"section": _INPUTS, "label": "T2 (follow-up)"}, + # Segments, not a drop-down: two layouts of equal standing, both on screen, + # and the client puts it above the inputs it switches between. + "input_layout": { + "section": _INPUTS, "label": "Data organisation", "ui": "segmented", + "option_help": dict(catalogs.LAYOUT_HELP), + }, + "patients": { + "section": _INPUTS, "label": "Patient folders", "visible_when": _BY_PATIENT, + "required_when": _BY_PATIENT, + }, + "t1": { + "section": _INPUTS, "label": "T1 (baseline)", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, + "t2": { + "section": _INPUTS, "label": "T2 (follow-up)", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, # NOT "Mode". The facade publishes its own `mode` -- the modality -- and # labels it "Mode" too, so a panel reached through AREG shows two dropdowns # side by side under the same word: one saying CBCT, the next saying diff --git a/tools/AREG/AREG_CBCT/tests/test_patient_folders.py b/tools/AREG/AREG_CBCT/tests/test_patient_folders.py new file mode 100644 index 0000000..ad401da --- /dev/null +++ b/tools/AREG/AREG_CBCT/tests/test_patient_folders.py @@ -0,0 +1,56 @@ +"""A cohort kept as one folder per patient reaches the engine as T1 and T2.""" + +import inspect +import os +import typing + +import pytest +import sadt_areg_cbct +from sadt_areg_common import catalogs +from sadt_areg_common.errors import ToolInputError + + +def touch(path): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as handle: + handle.write("x") + + +@pytest.fixture +def seen(monkeypatch): + """What the engine was handed, read before the sorted folders are removed.""" + found = {} + + def main(t1, t2, **_rest): + found["t1"] = sorted(os.listdir(t1)) + found["t2"] = sorted(os.listdir(t2)) + found["scratch"] = os.path.dirname(t1) + + monkeypatch.setattr(sadt_areg_cbct, "main", main) + return found + + +def test_the_layouts_are_the_catalogs(): + hint = inspect.signature(sadt_areg_cbct.run).parameters["input_layout"].annotation + assert list(typing.get_args(hint)) == list(catalogs.LAYOUT_CHOICES) + + +def test_a_patient_folder_reaches_the_engine_as_its_two_timepoints(tmp_path, seen): + root = tmp_path / "patients" / "case_042" + touch(root / "T1" / "scan.nii.gz") + touch(root / "T2" / "scan.nii.gz") + touch(root / "T1" / "scan_seg.nii.gz") + + sadt_areg_cbct.run( + input_layout=catalogs.LAYOUT_BY_PATIENT, patients=tmp_path / "patients", + output_dir=tmp_path / "out", + ) + + assert seen["t1"] == ["case_042_T1.nii.gz"] + assert seen["t2"] == ["case_042_T2.nii.gz"] + assert not os.path.exists(seen["scratch"]) + + +def test_the_per_type_layout_still_needs_both_folders(tmp_path, seen): + with pytest.raises(ToolInputError, match="'t2' is empty"): + sadt_areg_cbct.run(t1=tmp_path, output_dir=tmp_path / "out") diff --git a/tools/AREG/AREG_IOS/src/sadt_areg_ios/__init__.py b/tools/AREG/AREG_IOS/src/sadt_areg_ios/__init__.py index 3499966..c95a1de 100644 --- a/tools/AREG/AREG_IOS/src/sadt_areg_ios/__init__.py +++ b/tools/AREG/AREG_IOS/src/sadt_areg_ios/__init__.py @@ -14,9 +14,12 @@ tools reached through the supervisor; see `tools.py`. """ +import shutil from pathlib import Path from typing import Literal +from sadt_areg_common import patient_folders + from .dispatch import main @@ -45,10 +48,41 @@ def match(roots): return batching.pairs_by_name({"t1": t1, "t2": t2}, match, known_extensions=SURFACE_EXTENSIONS) +def _is_mesh(name: str) -> bool: + # Imported here: `surfaces` loads vtk, and CI imports this module on every + # PR to publish the schema. + from .surfaces import is_surface_file + + return is_surface_file(name) + + +_ROLE = patient_folders.by_timepoint(_is_mesh) + + +def _filed_as(patient: str, role: str, relative: str, index: int) -> str: + """`_T1_U.vtk`: the patient, the timepoint and the jaw the pairing reads. + + A mesh whose name says no jaw is filed without one, and reported by the + pairing as it is in the per-type layout. + """ + name = relative.replace("\\", "/").rsplit("/", 1)[-1] + jaw = patient_folders.jaw_of(name) or f"mesh{index}" + return f"{patient}_{role.upper()}_{jaw}{patient_folders.extension_of(name)}" + + def run( - t1: Path, - t2: Path, - output_dir: Path, + # First, because it decides which of the next three the panel asks for. + # Spelled out because `Literal` takes literals only; a test asserts it + # against `catalogs.LAYOUT_CHOICES`. + input_layout: Literal["One folder per type", "One folder per patient"] = "One folder per type", + # Not required any more, and not because they are optional: which of + # `t1`/`t2` or `patients` is needed depends on `input_layout`, and a field + # the panel marks required in the layout that does not read it would + # refuse to apply. `patient_folders.resolve` refuses what is missing. + t1: Path = "", + t2: Path = "", + patients: Path = "", + output_dir: Path = "", # "From the data" is the default and names no mode: `dispatch.derive_automation` # reads it off the meshes -- ones that already carry their tooth labels are # Semi-Automated, ones that do not are Fully-Automated. Naming either is an @@ -73,6 +107,12 @@ def run( """Register a follow-up intraoral scan onto its baseline, so the two compare. Args: + input_layout: How the files are organised: one folder for each type + of file (T1 and T2, or intraoral scans and CBCT) holding every + patient, or one folder for each patient holding all of that + patient's files. Rest the pointer on each option for an example. + patients: The folder of patient folders, in the one-folder-per-patient + layout. Each patient's folder name names its results. t1: The baseline meshes -- one surface or a folder of them, searched recursively. Each must say its jaw in its name. t2: The follow-up meshes, paired to T1 by patient key. @@ -104,19 +144,27 @@ def run( # torch, pytorch3d and monai are imported inside the engine: CI imports this # module on every PR to publish the schema, and that must not cost a CUDA # stack. - return main( - t1=t1, - t2=t2, - output_dir=output_dir, - automation=automation, - ios_reference=ios_reference, - ios_patch=patch, - registration_model=registration_model, - crown_model=crown_model, - mgl_model=mgl_model, - mgl_landmarks=mgl_landmarks, - mgl_patch_height=mgl_patch_height, - output_suffix=output_suffix, - sup=sup, - data_root=data_root, + folders, scratch = patient_folders.resolve( + input_layout, patients, {"t1": t1, "t2": t2}, output_dir, _ROLE, _filed_as, ) + try: + result = main( + t1=folders["t1"], + t2=folders["t2"], + output_dir=output_dir, + automation=automation, + ios_reference=ios_reference, + ios_patch=patch, + registration_model=registration_model, + crown_model=crown_model, + mgl_model=mgl_model, + mgl_landmarks=mgl_landmarks, + mgl_patch_height=mgl_patch_height, + output_suffix=output_suffix, + sup=sup, + data_root=data_root, + ) + finally: + if scratch: + shutil.rmtree(scratch, ignore_errors=True) + return result diff --git a/tools/AREG/AREG_IOS/src/sadt_areg_ios/layout.py b/tools/AREG/AREG_IOS/src/sadt_areg_ios/layout.py index 7740d4b..f418f21 100644 --- a/tools/AREG/AREG_IOS/src/sadt_areg_ios/layout.py +++ b/tools/AREG/AREG_IOS/src/sadt_areg_ios/layout.py @@ -8,6 +8,15 @@ from sadt_areg_common import catalogs _INPUTS = "Inputs" + +# Which of the two input shapes the panel asks for. The per-type fields and the +# folder of patients are never shown together: a reader who filled both would +# not know which one the run read. Each is REQUIRED in its own layout, which +# the signature cannot say -- both are optional there, since either may be +# the one not sent. +_BY_TYPE = {"input_layout": [catalogs.LAYOUT_BY_TYPE]} +_BY_PATIENT = {"input_layout": [catalogs.LAYOUT_BY_PATIENT]} + _REGISTRATION = "Registration" _OUTPUTS = "Outputs" @@ -56,8 +65,24 @@ # end, and a ticked step pauses the run and opens what it produced in VISU. "stop_after": {"label": "Stop after"}, - "t1": {"section": _INPUTS, "label": "T1 (baseline)"}, - "t2": {"section": _INPUTS, "label": "T2 (follow-up)"}, + # Segments, not a drop-down: two layouts of equal standing, both on screen, + # and the client puts it above the inputs it switches between. + "input_layout": { + "section": _INPUTS, "label": "Data organisation", "ui": "segmented", + "option_help": dict(catalogs.LAYOUT_HELP), + }, + "patients": { + "section": _INPUTS, "label": "Patient folders", "visible_when": _BY_PATIENT, + "required_when": _BY_PATIENT, + }, + "t1": { + "section": _INPUTS, "label": "T1 (baseline)", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, + "t2": { + "section": _INPUTS, "label": "T2 (follow-up)", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, # NOT "Mode". The facade publishes its own `mode` -- the modality -- and # labels it "Mode" too, so a panel reached through AREG shows two dropdowns # side by side under the same word: one saying CBCT, the next saying diff --git a/tools/AREG/AREG_IOS/tests/test_patient_folders.py b/tools/AREG/AREG_IOS/tests/test_patient_folders.py new file mode 100644 index 0000000..4c2f381 --- /dev/null +++ b/tools/AREG/AREG_IOS/tests/test_patient_folders.py @@ -0,0 +1,56 @@ +"""A cohort kept as one folder per patient reaches the engine as T1 and T2.""" + +import inspect +import os +import typing + +import pytest +import sadt_areg_ios +from sadt_areg_common import catalogs +from sadt_areg_common.errors import ToolInputError + + +def touch(path): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as handle: + handle.write("x") + + +@pytest.fixture +def seen(monkeypatch): + """What the engine was handed, read before the sorted folders are removed.""" + found = {} + + def main(t1, t2, **_rest): + found["t1"] = sorted(os.listdir(t1)) + found["t2"] = sorted(os.listdir(t2)) + found["scratch"] = os.path.dirname(t1) + + monkeypatch.setattr(sadt_areg_ios, "main", main) + return found + + +def test_the_layouts_are_the_catalogs(): + hint = inspect.signature(sadt_areg_ios.run).parameters["input_layout"].annotation + assert list(typing.get_args(hint)) == list(catalogs.LAYOUT_CHOICES) + + +def test_a_patient_folder_reaches_the_engine_as_its_two_timepoints(tmp_path, seen): + root = tmp_path / "patients" / "case_042" + touch(root / "T1" / "scan_U.vtk") + touch(root / "case_042_T2_L.vtk") + touch(root / "notes.txt") + + sadt_areg_ios.run( + input_layout=catalogs.LAYOUT_BY_PATIENT, patients=tmp_path / "patients", + output_dir=tmp_path / "out", + ) + + assert seen["t1"] == ["case_042_T1_U.vtk"] + assert seen["t2"] == ["case_042_T2_L.vtk"] + assert not os.path.exists(seen["scratch"]) + + +def test_the_per_type_layout_still_needs_both_folders(tmp_path, seen): + with pytest.raises(ToolInputError, match="'t2' is empty"): + sadt_areg_ios.run(t1=tmp_path, output_dir=tmp_path / "out") diff --git a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py index a6e18ef..eb1bdf7 100644 --- a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py +++ b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/__init__.py @@ -17,7 +17,10 @@ from pathlib import Path from typing import Literal -from sadt_areg_common import pairing +import shutil + +from sadt_areg_common import pairing, patient_folders +from sadt_areg_common.errors import ToolInputError from . import pipeline from .dispatch import main @@ -39,10 +42,56 @@ } +def _role(patient: str, relative: str): + """"ios" for a mesh, "cbct" for a volume, None for anything else. + + By the file's type alone: the two modalities are never the same kind of + file, so a patient folder needs no `IOS/` and `CBCT/` sub-folders -- though + it may have them. + """ + name = relative.replace("\\", "/").rsplit("/", 1)[-1] + if name.lower().endswith(pipeline.SURFACE_EXTENSIONS): + return "ios" + if pairing.is_scan_file(name): + return "cbct" + return None + + +def _filed_as(patient: str, role: str, relative: str, index: int) -> str: + """`_U.stl`, `_CBCT.nii.gz`: the names the pairing reads. + + The patient's folder name comes first, so `pipeline.patient_name` gives it + back as the output folder; the jaw stays, since that is how each arch finds + its landmarks. Patients are matched on the DIGITS of their names, so a + folder name needs one. + """ + if not any(c.isdigit() for c in patient): + raise ToolInputError( + "A patient folder's name carries no number. Patients are matched on " + "the digits of their names: name each folder with its patient's " + "number (e.g. patient001)." + ) + name = relative.replace("\\", "/").rsplit("/", 1)[-1] + extension = patient_folders.extension_of(name) + if role == "cbct": + return f"{patient}_CBCT{extension}" + jaw = patient_folders.jaw_of(name) + return f"{patient}_{jaw or f'IOS{index}'}{extension}" + + def run( - ios: Path, - cbct: Path, - output_dir: Path, + # First, because it decides which of the next three the panel asks for. + # Spelled out because `Literal` takes literals only; a test asserts it + # against `catalogs.LAYOUT_CHOICES`. + input_layout: Literal["One folder per type", "One folder per patient"] = "One folder per type", + # Not required any more, and not because they are optional: which of + # `ios`/`cbct` or `patients` is needed depends on `input_layout`, and a + # field the panel marks required in the layout that does not read it would + # refuse to apply. `patient_folders.resolve` refuses what is missing. + ios: Path = "", + cbct: Path = "", + patients: Path = "", + output_dir: Path = "", # "From the data" is the default and names no mode: `dispatch.derive_automation` # reads it off the request -- both landmark sets supplied is Registration, # which predicts nothing. Whether to orient the CBCT first is `orient_cbct_first` @@ -73,6 +122,13 @@ def run( """Register an intraoral scan onto a CBCT of the same patient. Args: + input_layout: How the files are organised: one folder for each type + of file (T1 and T2, or intraoral scans and CBCT) holding every + patient, or one folder for each patient holding all of that + patient's files. Rest the pointer on each option for an example. + patients: The folder of patient folders, in the one-folder-per-patient + layout. Each patient's folder name, with its number, names its + results. ios: The intraoral surfaces (.vtk/.stl), one folder, searched recursively. Upper and lower arches are registered separately and matched to their landmarks by the jaw token in the name. @@ -111,12 +167,20 @@ def run( The output directory. """ output_dir = Path(output_dir) - main( - ios=ios, cbct=cbct, output_dir=output_dir, automation=automation, - ios_landmarks=ios_landmarks, cbct_landmarks=cbct_landmarks, - cbct_reference=cbct_reference, landmark_model=landmark_model, - ios_landmark_model=ios_landmark_model, crown_model=crown_model, - max_dist=max_dist, output_suffix=output_suffix, - orient_cbct_first=orient_cbct_first, sup=sup, + folders, scratch = patient_folders.resolve( + input_layout, patients, {"ios": ios, "cbct": cbct}, output_dir, _role, _filed_as, ) + try: + main( + ios=folders["ios"], cbct=folders["cbct"], output_dir=output_dir, + automation=automation, + ios_landmarks=ios_landmarks, cbct_landmarks=cbct_landmarks, + cbct_reference=cbct_reference, landmark_model=landmark_model, + ios_landmark_model=ios_landmark_model, crown_model=crown_model, + max_dist=max_dist, output_suffix=output_suffix, + orient_cbct_first=orient_cbct_first, sup=sup, + ) + finally: + if scratch: + shutil.rmtree(scratch, ignore_errors=True) return output_dir diff --git a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/layout.py b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/layout.py index ee30124..da42ca4 100644 --- a/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/layout.py +++ b/tools/AREG/AREG_IOSCBCT/src/sadt_areg_ioscbct/layout.py @@ -3,6 +3,15 @@ from sadt_areg_common import catalogs _INPUTS = "Inputs" + +# Which of the two input shapes the panel asks for. The per-type fields and the +# folder of patients are never shown together: a reader who filled both would +# not know which one the run read. Each is REQUIRED in its own layout, which +# the signature cannot say -- both are optional there, since either may be +# the one not sent. +_BY_TYPE = {"input_layout": [catalogs.LAYOUT_BY_TYPE]} +_BY_PATIENT = {"input_layout": [catalogs.LAYOUT_BY_PATIENT]} + # "Advanced", not "Landmarks", and the name does two things at once. # # It is what the section IS: landmarks a caller already has are the escape hatch @@ -69,8 +78,24 @@ # end, and a ticked step pauses the run and opens what it produced in VISU. "stop_after": {"label": "Stop after"}, - "ios": {"section": _INPUTS, "label": "Intraoral scans"}, - "cbct": {"section": _INPUTS, "label": "CBCT volumes"}, + # Segments, not a drop-down: two layouts of equal standing, both on screen, + # and the client puts it above the inputs it switches between. + "input_layout": { + "section": _INPUTS, "label": "Data organisation", "ui": "segmented", + "option_help": dict(catalogs.LAYOUT_HELP), + }, + "patients": { + "section": _INPUTS, "label": "Patient folders", "visible_when": _BY_PATIENT, + "required_when": _BY_PATIENT, + }, + "ios": { + "section": _INPUTS, "label": "Intraoral scans", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, + "cbct": { + "section": _INPUTS, "label": "CBCT volumes", + "visible_when": _BY_TYPE, "required_when": _BY_TYPE, + }, # NOT "Mode". The facade publishes its own `mode` -- the modality -- and # labels it "Mode" too, so a panel reached through AREG shows two dropdowns # side by side under the same word: one saying CBCT, the next saying diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_patient_folders.py b/tools/AREG/AREG_IOSCBCT/tests/test_patient_folders.py new file mode 100644 index 0000000..bc8d8d3 --- /dev/null +++ b/tools/AREG/AREG_IOSCBCT/tests/test_patient_folders.py @@ -0,0 +1,73 @@ +"""A cohort kept as one folder per patient registers like one kept per type.""" + +import inspect +import os +import shutil +import typing + +import pytest +from conftest import cohort, read_report, tree_of +from sadt_areg_common import catalogs +from sadt_areg_common.errors import ToolInputError +from sadt_areg_ioscbct import run + + +def as_patient_folders(tmp_path, subfolders=False): + """The cohort `cohort()` wrote, moved to `patients//`.""" + root = tmp_path / "patients" / "P001" + for side in ("ios", "cbct"): + destination = root / side.upper() if subfolders else root + destination.mkdir(parents=True, exist_ok=True) + for name in os.listdir(tmp_path / side): + shutil.move(str(tmp_path / side / name), str(destination / name)) + return tmp_path / "patients" + + +def registration(tmp_path, **arguments): + return run( + output_dir=tmp_path / "out", automation="Registration", + ios_landmarks=tmp_path / "ios_lm", cbct_landmarks=tmp_path / "cbct_lm", + **arguments, + ) + + +def test_the_layouts_are_the_catalogs(): + hint = inspect.signature(run).parameters["input_layout"].annotation + assert list(typing.get_args(hint)) == list(catalogs.LAYOUT_CHOICES) + + +@pytest.mark.parametrize("subfolders", [False, True]) +def test_a_patient_folder_registers_into_a_folder_of_its_name(tmp_path, subfolders): + cohort(tmp_path) + patients = as_patient_folders(tmp_path, subfolders=subfolders) + + registration(tmp_path, input_layout=catalogs.LAYOUT_BY_PATIENT, patients=patients) + + assert tree_of(tmp_path / "out") == sorted([ + "AREG_report.json", + os.path.join("P001", "P001_U_Reg.vtk"), + os.path.join("P001", "P001_U_Reg_matrix.npy"), + ]) + assert read_report(tmp_path / "out")["patients"]["P001"]["status"] == "ok" + + +def test_the_sorted_copies_are_removed_and_never_handed_back(tmp_path): + cohort(tmp_path) + patients = as_patient_folders(tmp_path) + registration(tmp_path, input_layout=catalogs.LAYOUT_BY_PATIENT, patients=patients) + assert not [name for name in os.listdir(tmp_path) if name.startswith("areg-patients-")] + + +def test_a_patient_folder_with_no_number_is_refused(tmp_path): + """Patients are matched on digits; a name without one cannot be.""" + cohort(tmp_path) + patients = as_patient_folders(tmp_path) + os.rename(patients / "P001", patients / "no_number") + with pytest.raises(ToolInputError, match="carries no number"): + registration(tmp_path, input_layout=catalogs.LAYOUT_BY_PATIENT, patients=patients) + + +def test_the_per_type_layout_still_needs_both_folders(tmp_path): + cohort(tmp_path) + with pytest.raises(ToolInputError, match="'cbct' is empty"): + registration(tmp_path, ios=tmp_path / "ios") diff --git a/tools/AREG/AREG_IOSCBCT/tests/test_run.py b/tools/AREG/AREG_IOSCBCT/tests/test_run.py index 8b565a8..57a27fa 100644 --- a/tools/AREG/AREG_IOSCBCT/tests/test_run.py +++ b/tools/AREG/AREG_IOSCBCT/tests/test_run.py @@ -457,9 +457,12 @@ def test_every_layout_condition_names_a_mode_this_tool_has(): if not condition: continue assert set(condition) <= set(inspect.signature(run).parameters), argument - wanted = condition["automation"] - for mode in (wanted if isinstance(wanted, list) else [wanted]): - assert mode in offered, (argument, mode) + # Every condition's values, and not only automation's: a misspelt + # layout hides a field just as surely as a misspelt mode. + values = {"automation": offered, "input_layout": set(catalogs.LAYOUT_CHOICES)} + for name, wanted in condition.items(): + for value in (wanted if isinstance(wanted, list) else [wanted]): + assert value in values[name], (argument, name, value) def test_the_landmark_folders_are_hidden_in_the_modes_that_overwrite_them(): diff --git a/tools/AREG/common/src/sadt_areg_common/catalogs.py b/tools/AREG/common/src/sadt_areg_common/catalogs.py index 4d47c9b..ba1889f 100644 --- a/tools/AREG/common/src/sadt_areg_common/catalogs.py +++ b/tools/AREG/common/src/sadt_areg_common/catalogs.py @@ -279,3 +279,62 @@ def region_name(code: str) -> str: # The jaw each patch registers. PATCH_JAW = {PATCH_PALATE: JAW_UPPER, PATCH_MGL: JAW_LOWER} + + +# --------------------------------------------------------------------------- +# Input layout +# --------------------------------------------------------------------------- +# How the caller's files are organised. A cohort kept as one folder per type +# (`T1/` and `T2/`, or `IOS/` and `CBCT/`) is the layout every engine reads; +# one kept as one folder per patient (`/T1/`, `/CBCT/`, or +# the files side by side) is sorted into it first, by `patient_folders`. + +LAYOUT_BY_TYPE = "One folder per type" +LAYOUT_BY_PATIENT = "One folder per patient" + +LAYOUT_CHOICES = {LAYOUT_BY_TYPE: True, LAYOUT_BY_PATIENT: False} + +# What each layout looks like on disk, shown when the pointer rests on that +# option. One text for the three engines: the facade publishes one help per +# option whatever the mode, so each tree shows both the T1/T2 and the IOS/CBCT +# shape. Plain text, line breaks kept. +LAYOUT_HELP = { + LAYOUT_BY_TYPE: ( + "One folder for each type of file, every patient inside it.\n" + "Patients are matched across the two folders by their file names.\n" + "\n" + "CBCT or IOS registration (T1 and T2):\n" + " T1/\n" + " patient001_T1.nii.gz\n" + " patient002_T1.nii.gz\n" + " T2/\n" + " patient001_T2.nii.gz\n" + " patient002_T2.nii.gz\n" + "\n" + "IOS to CBCT:\n" + " IOS/\n" + " patient001_U.stl\n" + " patient001_L.stl\n" + " CBCT/\n" + " patient001_CBCT.nii.gz" + ), + LAYOUT_BY_PATIENT: ( + "One folder for each patient, holding all of that patient's files.\n" + "The folder's name is the patient's name, and names the results.\n" + "\n" + "CBCT or IOS registration (T1 and T2):\n" + " patients/\n" + " patient001/\n" + " T1/ scan.nii.gz\n" + " T2/ scan.nii.gz\n" + " patient002/\n" + " patient002_T1.nii.gz\n" + " patient002_T2.nii.gz\n" + "\n" + "IOS to CBCT (sub-folders optional):\n" + " patients/\n" + " patient001/\n" + " IOS/ upper.stl lower.stl\n" + " CBCT/ scan.nii.gz" + ), +} diff --git a/tools/AREG/common/src/sadt_areg_common/patient_folders.py b/tools/AREG/common/src/sadt_areg_common/patient_folders.py new file mode 100644 index 0000000..2ee3302 --- /dev/null +++ b/tools/AREG/common/src/sadt_areg_common/patient_folders.py @@ -0,0 +1,202 @@ +"""Read a cohort kept as one folder per patient. + +Every AREG engine reads its two inputs as two folders, one per type: `t1` and +`t2`, or `ios` and `cbct`. Cohorts are often kept the other way round, one +folder per patient holding both (`/T1/` and `/T2/`, or +`/IOS/` and `/CBCT/`), and asking for two folders then meant +reorganising the data by hand before every run. + +This module does that reorganising, and nothing else: it sorts each patient's +files into one folder per type, under names that carry the patient's folder +name, and hands the engine those folders. Everything downstream -- the +pairing, the supervised tools, the output folders -- is then exactly what a +per-type request runs, which is what makes the two layouts give the same +answer. + +The patient IS its folder. A file's own name is not trusted to say whose it is: +`/T1/scan.nii.gz` is that patient's T1 whatever it is called, which +is the reason to keep data this way in the first place. +""" + +import os +import shutil +import tempfile + +from . import catalogs, pairing +from .errors import ToolInputError + +_TIMEPOINTS = {"t1": "T1", "t2": "T2"} + + +def timepoint_of(relative: str): + """"T1", "T2" or None for a file at `relative` inside a patient's folder. + + Read from a folder on the way named after the timepoint, and from a + timepoint word in the file's own name; either is enough. Two different + answers is no answer: a file in `T1/` whose name says T2 is left out, not + guessed at. + """ + *folders, name = relative.replace("\\", "/").split("/") + by_folder = {_TIMEPOINTS[f.lower()] for f in folders if f.lower() in _TIMEPOINTS} + by_name = {_TIMEPOINTS[t] for t in pairing.tokens(pairing.split_scan_extension(name)[0]) + if t in _TIMEPOINTS} + found = by_folder | by_name + return found.pop() if len(found) == 1 else None + + +def by_timepoint(accept, skip=()): + """A `classify` for the two longitudinal engines: "t1", "t2" or None. + + `accept(name)` says which files the engine reads at all; a file whose name + holds a word in `skip` is left out -- a segmentation kept beside its scan + is not a second T1. + """ + skip = set(skip) + + def classify(patient, relative): + name = relative.replace("\\", "/").rsplit("/", 1)[-1] + if not accept(name): + return None + if skip & set(pairing.tokens(pairing.split_scan_extension(name)[0])): + return None + timepoint = timepoint_of(relative) + return timepoint.lower() if timepoint else None + + return classify + + +def jaw_of(name: str): + """"U", "L" or None, from the jaw word in a mesh's name.""" + jaws = {catalogs.JAW_TOKENS[t] for t in pairing.tokens(os.path.splitext(name)[0]) + if t in catalogs.JAW_TOKENS} + if jaws == {catalogs.JAW_UPPER}: + return "U" + if jaws == {catalogs.JAW_LOWER}: + return "L" + return None + + +def patients_in(root: str) -> list: + """The patient folders of `root`, sorted. Hidden entries are not patients.""" + if not root or not os.path.isdir(root): + raise ToolInputError( + "'patients' must be a folder holding one sub-folder per patient." + ) + found = sorted( + entry for entry in os.listdir(root) + if not entry.startswith(".") and os.path.isdir(os.path.join(root, entry)) + ) + if not found: + raise ToolInputError( + "'patients' holds no sub-folder. In this layout each patient has a " + "folder of its own, holding both of that patient's inputs." + ) + return found + + +def files_of(folder: str) -> list: + """Every file under one patient's folder, as paths relative to it, sorted.""" + found = [] + for directory, directories, names in os.walk(folder): + directories[:] = sorted(d for d in directories if not d.startswith(".")) + for name in sorted(names): + if not name.startswith("."): + found.append(os.path.relpath(os.path.join(directory, name), folder)) + return found + + +def place(source: str, destination: str) -> None: + """Put `source` at `destination` without copying a scan when it can be helped. + + A hard link costs nothing and is a real file to every reader, which a + symbolic link is not to a tool that refuses to follow one. It needs both + paths on one filesystem; a copy is the fallback. + """ + os.makedirs(os.path.dirname(destination), exist_ok=True) + try: + os.link(source, destination) + except OSError: + shutil.copy2(source, destination) + + +def extension_of(name: str) -> str: + """The whole extension, `.nii.gz` included.""" + _stem, extension = pairing.split_scan_extension(name) + if extension: + return extension + return os.path.splitext(name)[1] + + +def sort(root: str, work_dir: str, roles: tuple, classify, rename) -> dict: + """Sort a per-patient cohort into one folder per role under `work_dir`. + + `roles` names the folders made (`("t1", "t2")`). `classify(patient, + relative)` gives the role of one file, or None to leave it out; `rename( + patient, role, relative, index)` gives the name it is filed under, `index` + counting that patient's files of that role from 1. Two files given one name + are refused rather than one silently replacing the other. + + Returns `{role: folder}`. A patient with no file for a role is left for the + engine to report as unpaired, the way it reports one in a per-type + request. + """ + folders = {role: os.path.join(work_dir, role) for role in roles} + for folder in folders.values(): + os.makedirs(folder, exist_ok=True) + + filed = 0 + for patient in patients_in(root): + counts = dict.fromkeys(roles, 0) + for relative in files_of(os.path.join(root, patient)): + role = classify(patient, relative) + if role not in folders: + continue + counts[role] += 1 + destination = os.path.join(folders[role], rename(patient, role, relative, counts[role])) + if os.path.exists(destination): + # Counts, never the names: a patient folder is the caller's own + # identifier, and this message reaches the server's log. + raise ToolInputError( + f"Two files of one patient would be read as the same " + f"{role.upper()} input. Keep one file per input, or name " + "them apart (for an arch, by its jaw: U or L)." + ) + place(os.path.join(root, patient, relative), destination) + filed += 1 + if not filed: + raise ToolInputError( + "No usable file in any patient folder of 'patients'." + ) + return folders + + +def resolve(layout: str, patients, given: dict, output_dir, classify, rename) -> tuple: + """`({role: folder}, scratch)`: the per-type folders an engine reads. + + `given` is the engine's own per-type arguments (`{"t1": ..., "t2": ...}`), + read as they are in the per-type layout. In the per-patient one they are + ignored and `patients` is sorted into a scratch folder beside + `output_dir` -- beside it, not in it, so nothing of it is handed back, and + on the filesystem the inputs were unpacked on, so the files are linked + rather than copied. `scratch` is that folder, for the caller to remove, or + None. + """ + if layout == catalogs.LAYOUT_BY_PATIENT: + parent = os.path.dirname(os.path.abspath(str(output_dir))) if output_dir else None + scratch = tempfile.mkdtemp(prefix="areg-patients-", dir=parent) + try: + return sort(str(patients or ""), scratch, tuple(given), classify, rename), scratch + except BaseException: + shutil.rmtree(scratch, ignore_errors=True) + raise + if layout != catalogs.LAYOUT_BY_TYPE: + raise ToolInputError( + f"Unknown input layout {layout!r}: choose one of {', '.join(catalogs.LAYOUT_CHOICES)}." + ) + missing = [role for role, value in given.items() if not str(value or "").strip()] + if missing: + raise ToolInputError( + f"'{missing[0]}' is empty. Send one folder per type, or choose " + f"\"{catalogs.LAYOUT_BY_PATIENT}\" and send the folder of patients." + ) + return {role: str(value) for role, value in given.items()}, None diff --git a/tools/AREG/common/tests/test_patient_folders.py b/tools/AREG/common/tests/test_patient_folders.py new file mode 100644 index 0000000..ad43e7f --- /dev/null +++ b/tools/AREG/common/tests/test_patient_folders.py @@ -0,0 +1,156 @@ +"""A cohort kept as one folder per patient, sorted into one folder per type.""" + +import os + +import pytest +from sadt_areg_common import catalogs, pairing, patient_folders +from sadt_areg_common.errors import ToolInputError + + +def touch(path): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as handle: + handle.write("x") + return path + + +def tree(root): + return sorted( + os.path.relpath(os.path.join(directory, name), root) + for directory, _dirs, names in os.walk(root) for name in names + ) + + +_CBCT = patient_folders.by_timepoint(pairing.is_scan_file, skip=catalogs.MASK_TOKENS) + + +def _cbct_name(patient, role, relative, index): + return f"{patient}_{role.upper()}{patient_folders.extension_of(relative)}" + + +# --------------------------------------------------------------------------- +# What a file is +# --------------------------------------------------------------------------- + +def test_the_timepoint_comes_from_the_folder_or_the_name(): + assert patient_folders.timepoint_of("T1/scan.nii.gz") == "T1" + assert patient_folders.timepoint_of("t2/scan.nii.gz") == "T2" + assert patient_folders.timepoint_of("case_042_T2.nii.gz") == "T2" + assert patient_folders.timepoint_of("scan.nii.gz") is None + + +def test_a_folder_and_a_name_that_disagree_give_no_timepoint(): + """Guessing between them is how a baseline gets registered onto itself.""" + assert patient_folders.timepoint_of("T1/scan_T2.nii.gz") is None + + +def test_the_jaw_comes_from_the_name(): + assert patient_folders.jaw_of("scan_U.stl") == "U" + assert patient_folders.jaw_of("Lower.vtk") == "L" + assert patient_folders.jaw_of("scan.vtk") is None + + +# --------------------------------------------------------------------------- +# Sorting +# --------------------------------------------------------------------------- + +def test_each_patients_files_are_filed_under_their_folders_name(tmp_path): + touch(tmp_path / "in" / "case_042" / "T1" / "scan.nii.gz") + touch(tmp_path / "in" / "case_042" / "T2" / "scan.nii.gz") + touch(tmp_path / "in" / "case_043" / "case_043_T1.nii.gz") + touch(tmp_path / "in" / "case_043" / "case_043_T2.nii.gz") + + folders = patient_folders.sort( + str(tmp_path / "in"), str(tmp_path / "work"), ("t1", "t2"), _CBCT, _cbct_name + ) + assert tree(folders["t1"]) == ["case_042_T1.nii.gz", "case_043_T1.nii.gz"] + assert tree(folders["t2"]) == ["case_042_T2.nii.gz", "case_043_T2.nii.gz"] + + +def test_what_the_engine_does_not_read_is_left_out(tmp_path): + """A segmentation beside its scan is not a second T1, and a note is not a + scan at all.""" + touch(tmp_path / "in" / "case_042" / "T1" / "scan.nii.gz") + touch(tmp_path / "in" / "case_042" / "T1" / "scan_seg.nii.gz") + touch(tmp_path / "in" / "case_042" / "notes.txt") + + folders = patient_folders.sort( + str(tmp_path / "in"), str(tmp_path / "work"), ("t1", "t2"), _CBCT, _cbct_name + ) + assert tree(folders["t1"]) == ["case_042_T1.nii.gz"] + assert tree(folders["t2"]) == [] + + +def test_two_files_filed_under_one_name_are_refused(tmp_path): + """One replacing the other silently would register whichever came last.""" + touch(tmp_path / "in" / "case_042" / "T1" / "a.nii.gz") + touch(tmp_path / "in" / "case_042" / "T1" / "b.nii.gz") + + with pytest.raises(ToolInputError, match="same T1 input"): + patient_folders.sort( + str(tmp_path / "in"), str(tmp_path / "work"), ("t1", "t2"), _CBCT, _cbct_name + ) + + +def test_a_folder_with_no_patient_folder_is_refused(tmp_path): + touch(tmp_path / "in" / "case_042_T1.nii.gz") + with pytest.raises(ToolInputError, match="no sub-folder"): + patient_folders.sort( + str(tmp_path / "in"), str(tmp_path / "work"), ("t1", "t2"), _CBCT, _cbct_name + ) + + +def test_the_files_are_the_callers_own_not_copies_that_differ(tmp_path): + source = touch(tmp_path / "in" / "case_042" / "T1" / "scan.nii.gz") + folders = patient_folders.sort( + str(tmp_path / "in"), str(tmp_path / "work"), ("t1", "t2"), _CBCT, _cbct_name + ) + with open(os.path.join(folders["t1"], "case_042_T1.nii.gz"), encoding="utf-8") as handle: + assert handle.read() == "x" + assert os.path.exists(source) + + +# --------------------------------------------------------------------------- +# resolve +# --------------------------------------------------------------------------- + +def test_the_per_type_layout_passes_its_folders_through(tmp_path): + folders, scratch = patient_folders.resolve( + catalogs.LAYOUT_BY_TYPE, "", {"t1": "/a", "t2": "/b"}, str(tmp_path / "out"), + _CBCT, _cbct_name, + ) + assert folders == {"t1": "/a", "t2": "/b"} and scratch is None + + +def test_the_per_type_layout_still_needs_both_folders(tmp_path): + with pytest.raises(ToolInputError, match="'t2' is empty"): + patient_folders.resolve( + catalogs.LAYOUT_BY_TYPE, "", {"t1": "/a", "t2": ""}, str(tmp_path / "out"), + _CBCT, _cbct_name, + ) + + +def test_the_per_patient_layout_sorts_beside_the_output_never_in_it(tmp_path): + """In the output folder, the sorted copies would be handed back.""" + touch(tmp_path / "in" / "case_042" / "T1" / "scan.nii.gz") + touch(tmp_path / "in" / "case_042" / "T2" / "scan.nii.gz") + (tmp_path / "out").mkdir() + + folders, scratch = patient_folders.resolve( + catalogs.LAYOUT_BY_PATIENT, str(tmp_path / "in"), {"t1": "", "t2": ""}, + str(tmp_path / "out"), _CBCT, _cbct_name, + ) + assert os.path.dirname(scratch) == str(tmp_path) + assert tree(folders["t1"]) == ["case_042_T1.nii.gz"] + assert os.listdir(tmp_path / "out") == [] + + +def test_a_refused_sort_leaves_no_scratch_folder_behind(tmp_path): + (tmp_path / "in").mkdir() + (tmp_path / "out").mkdir() + with pytest.raises(ToolInputError): + patient_folders.resolve( + catalogs.LAYOUT_BY_PATIENT, str(tmp_path / "in"), {"t1": "", "t2": ""}, + str(tmp_path / "out"), _CBCT, _cbct_name, + ) + assert sorted(os.listdir(tmp_path)) == ["in", "out"]