ADD: let AREG read a cohort kept as one folder per patient, chosen beside the per-type layout

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(

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:
+    """`<patient>_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

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

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")

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:
+    """`<patient>_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