Skip to content

Commit 1e51d03

Browse files
authored
Drop tissue-mask transforms from longitudinal anat (Stage 4 of #301) (#314)
* Drop tissue-mask transforms from longitudinal anat (Stage 4 of #301) Tissue masks (csf/gm/wm) were being warped into longitudinal template space even though nothing downstream consumes them there; per Jason's handover note #5 this is wasted work. Removes the fields from `AnatomicalLongOutputs`, the `_xfm()` calls in `longitudinal_process`, and the matching resolve/export entries under `space-longitudinal`. Adds a unit test asserting the tissue masks are not produced, and trims the three obsolete rows from `docs/data_dictionary.md`. * Tighten Stage 4 tests (PR review fixups) - Flag the xfm filename quirk (space-longitudinal + from-/to- is redundant) with a comment pointing at handover note #18, so the hard-coded expected names are not mistaken for a spec. - Replace the Mock-based longitudinal anat outputs in the orchestration tests with a real AnatomicalLongOutputs NamedTuple so attribute typos fail loudly instead of silently returning a new Mock. - Add resolve_longitudinal_anat unit tests covering the happy path, brain_mask optionality, and bids_safe_label round-trip for session labels with non-alphanumerics.
1 parent 4904354 commit 1e51d03

5 files changed

Lines changed: 243 additions & 57 deletions

File tree

docs/data_dictionary.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,5 @@ Produced by the `rbc longitudinal` subcommand group (`template`, `anatomical`, `
131131
| -------------------------------------------------- | ------ | ------------------------------------------------------------------------- | ------------------------------------------ | ---------------------------- |
132132
| `*_space-longitudinal_desc-brain_T1w.nii.gz` | `T1w` | Skull-stripped brain image aligned to the subject's longitudinal template | ANTs registration to longitudinal template | 3D NIfTI |
133133
| `*_space-longitudinal_desc-T1w_mask.nii.gz` | `mask` | Brain mask in longitudinal template space | ANTs registration to longitudinal template | 3D NIfTI, binary mask |
134-
| `*_space-longitudinal_desc-csf_mask.nii.gz` | `mask` | CSF mask in longitudinal template space | ANTs registration to longitudinal template | 3D NIfTI, binary mask |
135-
| `*_space-longitudinal_desc-gm_mask.nii.gz` | `mask` | Gray matter mask in longitudinal template space | ANTs registration to longitudinal template | 3D NIfTI, binary mask |
136-
| `*_space-longitudinal_desc-wm_mask.nii.gz` | `mask` | White matter mask in longitudinal template space | ANTs registration to longitudinal template | 3D NIfTI, binary mask |
137134
| `*_from-T1w_to-longitudinal_mode-image_xfm.nii.gz` | `xfm` | Warp field mapping subject anatomy to the longitudinal template | ANTs registration | 3D NIfTI, displacement field |
138135
| `*_from-longitudinal_to-T1w_mode-image_xfm.nii.gz` | `xfm` | Inverse warp field mapping longitudinal template back to subject anatomy | ANTs registration | 3D NIfTI, displacement field |

src/rbc/bids/longitudinal/anatomical.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ def resolve_longitudinal_anat(
5252
),
5353
"brain": anat_q.expect(anat_df, suffix=Suffix.T1W, desc="brain"),
5454
"brain_mask": anat_q.find(anat_df, suffix=Suffix.MASK, desc="T1w"),
55-
"csf_mask": anat_q.find(anat_df, suffix=Suffix.MASK, desc="csf"),
56-
"gm_mask": anat_q.find(anat_df, suffix=Suffix.MASK, desc="gm"),
57-
"wm_mask": anat_q.find(anat_df, suffix=Suffix.MASK, desc="wm"),
5855
}
5956

6057

@@ -71,21 +68,6 @@ def export_longitudinal_anat(aex: Bids, outputs: AnatomicalLongOutputs) -> None:
7168
suffix=Suffix.MASK,
7269
desc="T1w",
7370
)
74-
aex.save(
75-
_require_file(outputs.csf_mask, "csf_mask"),
76-
suffix=Suffix.MASK,
77-
desc="csf",
78-
)
79-
aex.save(
80-
_require_file(outputs.gm_mask, "gm_mask"),
81-
suffix=Suffix.MASK,
82-
desc="gm",
83-
)
84-
aex.save(
85-
_require_file(outputs.wm_mask, "wm_mask"),
86-
suffix=Suffix.MASK,
87-
desc="wm",
88-
)
8971
aex.save(
9072
outputs.long_to_template_xfm,
9173
suffix="xfm",

src/rbc/workflows/longitudinal/anatomical.py

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,12 @@ class AnatomicalLongOutputs(NamedTuple):
2727
brain: Skull-stripped T1w brain warped to longitudinal template space.
2828
brain_mask: Binary brain mask warped to longitudinal template space,
2929
or *None* if not provided.
30-
csf_mask: CSF tissue mask warped to longitudinal template space,
31-
or *None* if not provided.
32-
gm_mask: GM tissue mask warped to longitudinal template space,
33-
or *None* if not provided.
34-
wm_mask: WM tissue mask warped to longitudinal template space,
35-
or *None* if not provided.
3630
long_to_template_xfm: Longitudinal template-to-MNI152 composite warp.
3731
template_to_long_xfm: MNI152-to-longitudinal template composite warp.
3832
"""
3933

4034
brain: Path
4135
brain_mask: Path | None
42-
csf_mask: Path | None
43-
gm_mask: Path | None
44-
wm_mask: Path | None
4536
long_to_template_xfm: Path
4637
template_to_long_xfm: Path
4738

@@ -52,9 +43,6 @@ def longitudinal_process(
5243
*,
5344
brain: Path,
5445
brain_mask: Path | None = None,
55-
csf_mask: Path | None = None,
56-
gm_mask: Path | None = None,
57-
wm_mask: Path | None = None,
5846
registration_template: Path = REGISTRATION_TEMPLATES.brain_1mm,
5947
) -> AnatomicalLongOutputs:
6048
"""Transform preprocessed anatomical outputs to longitudinal template space.
@@ -67,35 +55,28 @@ def longitudinal_process(
6755
subj_to_template_xfm: Subject-to-longitudinal-template composite warp.
6856
brain: Preprocessed brain image.
6957
brain_mask: Brain mask, if available.
70-
csf_mask: CSF partial volume mask, if available.
71-
gm_mask: Grey matter partial volume mask, if available.
72-
wm_mask: White matter partial volume mask, if available.
7358
registration_template: Brain template for ANTs registration.
7459
7560
Returns:
7661
:class:`AnatomicalLongOutputs` with all non-null inputs transformed to template
7762
space.
7863
"""
79-
80-
def _xfm(val: Path | None) -> Path | None:
81-
if val is None:
82-
return None
83-
return anat_transform(in_file=val, template=template, xfm=subj_to_template_xfm)
84-
8564
_logger.info("Transforming anatomical outputs to longitudinal template space")
8665
_logger.info("Registration of longitudinal template to standard-space (ANTs)")
8766
transforms = ants_registration(
8867
in_file=template,
8968
registration_template=registration_template,
9069
)
70+
brain_mask_out = (
71+
anat_transform(in_file=brain_mask, template=template, xfm=subj_to_template_xfm)
72+
if brain_mask is not None
73+
else None
74+
)
9175
return AnatomicalLongOutputs(
9276
brain=anat_transform(
9377
in_file=brain, template=template, xfm=subj_to_template_xfm
9478
),
95-
brain_mask=_xfm(brain_mask),
96-
csf_mask=_xfm(csf_mask),
97-
gm_mask=_xfm(gm_mask),
98-
wm_mask=_xfm(wm_mask),
79+
brain_mask=brain_mask_out,
9980
long_to_template_xfm=transforms.anat_to_template,
10081
template_to_long_xfm=transforms.template_to_anat,
10182
)
Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
"""Unit tests for ``rbc.bids.longitudinal.anatomical``."""
2+
3+
from __future__ import annotations
4+
5+
from typing import TYPE_CHECKING
6+
7+
import polars as pl
8+
9+
from rbc.bids.longitudinal.anatomical import (
10+
export_longitudinal_anat,
11+
resolve_longitudinal_anat,
12+
)
13+
from rbc.context import RunContext
14+
from rbc.workflows.longitudinal.anatomical import AnatomicalLongOutputs
15+
16+
if TYPE_CHECKING:
17+
from pathlib import Path
18+
19+
20+
def _make_long_outputs(workdir: Path) -> AnatomicalLongOutputs:
21+
"""Build a populated AnatomicalLongOutputs pointing at dummy files."""
22+
23+
def _dummy(name: str) -> Path:
24+
p = workdir / name
25+
p.write_bytes(b"\x00")
26+
return p
27+
28+
return AnatomicalLongOutputs(
29+
brain=_dummy("brain.nii.gz"),
30+
brain_mask=_dummy("brain_mask.nii.gz"),
31+
long_to_template_xfm=_dummy("long_to_tpl.nii.gz"),
32+
template_to_long_xfm=_dummy("tpl_to_long.nii.gz"),
33+
)
34+
35+
36+
class TestExportLongitudinalAnat:
37+
"""Tests for :func:`export_longitudinal_anat`."""
38+
39+
def test_writes_expected_files(self, tmp_path: Path) -> None:
40+
"""Exports brain, brain_mask, and the two xfms under space-longitudinal."""
41+
workdir = tmp_path / "work"
42+
workdir.mkdir()
43+
out_dir = tmp_path / "out"
44+
45+
ctx = RunContext(sub="01", ses="baseline", output_dir=out_dir)
46+
aex = ctx.bids(datatype="anat").derive(space="longitudinal")
47+
48+
export_longitudinal_anat(aex, _make_long_outputs(workdir))
49+
50+
saved = sorted(p.name for p in out_dir.rglob("*.*"))
51+
# Xfm filenames currently carry both ``space-longitudinal`` and the
52+
# ``from-…_to-…`` pair, which is redundant (Jason handover #18 flags
53+
# the ``from-`` workaround more broadly). We assert the current shape
54+
# so regressions are caught, but if that cleanup lands the expected
55+
# xfm names here should drop ``space-longitudinal_``.
56+
assert saved == [
57+
"sub-01_ses-baseline_space-longitudinal_desc-T1w_mask.nii.gz",
58+
"sub-01_ses-baseline_space-longitudinal_desc-brain_T1w.nii.gz",
59+
"sub-01_ses-baseline_space-longitudinal_from-MNI152NLin6Asym"
60+
"_to-longitudinal_mode-image_xfm.nii.gz",
61+
"sub-01_ses-baseline_space-longitudinal_from-longitudinal"
62+
"_to-MNI152NLin6Asym_mode-image_xfm.nii.gz",
63+
]
64+
65+
def test_tissue_masks_not_produced(self, tmp_path: Path) -> None:
66+
"""No csf/gm/wm masks are written under space-longitudinal (#5)."""
67+
workdir = tmp_path / "work"
68+
workdir.mkdir()
69+
out_dir = tmp_path / "out"
70+
71+
ctx = RunContext(sub="01", ses="baseline", output_dir=out_dir)
72+
aex = ctx.bids(datatype="anat").derive(space="longitudinal")
73+
74+
export_longitudinal_anat(aex, _make_long_outputs(workdir))
75+
76+
names = [p.name for p in out_dir.rglob("*.*")]
77+
for tissue in ("csf", "gm", "wm"):
78+
assert not any(f"desc-{tissue}_mask" in n for n in names), (
79+
f"Expected no space-longitudinal {tissue}_mask; got {names}"
80+
)
81+
82+
83+
def _anat_row(
84+
*,
85+
sub: str,
86+
ses: str,
87+
suffix: str,
88+
desc: str | None,
89+
ext: str = ".nii.gz",
90+
extra: list[dict[str, str]] | None = None,
91+
) -> dict[str, object]:
92+
"""Build a single BIDS-like row, including an ``extra_entities`` list."""
93+
desc_part = f"_desc-{desc}" if desc else ""
94+
path = f"sub-{sub}/ses-{ses}/anat/sub-{sub}_ses-{ses}{desc_part}_{suffix}{ext}"
95+
return {
96+
"datatype": "anat",
97+
"suffix": suffix,
98+
"ext": ext,
99+
"sub": sub,
100+
"ses": ses,
101+
"desc": desc,
102+
"root": "/data",
103+
"path": path,
104+
"extra_entities": extra or [],
105+
}
106+
107+
108+
def _df(*rows: dict[str, object]) -> pl.DataFrame:
109+
return pl.DataFrame(list(rows))
110+
111+
112+
class TestResolveLongitudinalAnat:
113+
"""Tests for :func:`resolve_longitudinal_anat`."""
114+
115+
def test_resolves_all_inputs(self, tmp_path: Path) -> None:
116+
"""Happy path: all four keys resolve to the expected paths."""
117+
anat_df = _df(
118+
_anat_row(sub="01", ses="baseline", suffix="T1w", desc="brain"),
119+
_anat_row(sub="01", ses="baseline", suffix="mask", desc="T1w"),
120+
)
121+
tpl_df = _df(
122+
_anat_row(sub="01", ses="longitudinal", suffix="T1w", desc=None),
123+
_anat_row(
124+
sub="01",
125+
ses="longitudinal",
126+
suffix="xfm",
127+
desc=None,
128+
ext=".txt",
129+
extra=[
130+
{"key": "from", "value": "baseline"},
131+
{"key": "to", "value": "longitudinal"},
132+
],
133+
),
134+
)
135+
136+
ctx = RunContext(sub="01", ses="baseline", output_dir=tmp_path)
137+
anat_q = ctx.bids(datatype="anat")
138+
tpl_q = anat_q.derive(ses="longitudinal")
139+
140+
resolved = resolve_longitudinal_anat(
141+
anat_q, tpl_q, anat_df, tpl_df, ses="baseline"
142+
)
143+
144+
assert set(resolved) == {
145+
"template",
146+
"subj_to_template_xfm",
147+
"brain",
148+
"brain_mask",
149+
}
150+
assert str(resolved["template"]).endswith("sub-01_ses-longitudinal_T1w.nii.gz")
151+
assert str(resolved["subj_to_template_xfm"]).endswith(
152+
"sub-01_ses-longitudinal_xfm.txt"
153+
)
154+
assert str(resolved["brain"]).endswith(
155+
"sub-01_ses-baseline_desc-brain_T1w.nii.gz"
156+
)
157+
assert str(resolved["brain_mask"]).endswith(
158+
"sub-01_ses-baseline_desc-T1w_mask.nii.gz"
159+
)
160+
161+
def test_missing_brain_mask_is_none(self, tmp_path: Path) -> None:
162+
"""brain_mask is optional (``.find``) — returns None when absent."""
163+
anat_df = _df(
164+
_anat_row(sub="01", ses="baseline", suffix="T1w", desc="brain"),
165+
)
166+
tpl_df = _df(
167+
_anat_row(sub="01", ses="longitudinal", suffix="T1w", desc=None),
168+
_anat_row(
169+
sub="01",
170+
ses="longitudinal",
171+
suffix="xfm",
172+
desc=None,
173+
ext=".txt",
174+
extra=[
175+
{"key": "from", "value": "baseline"},
176+
{"key": "to", "value": "longitudinal"},
177+
],
178+
),
179+
)
180+
181+
ctx = RunContext(sub="01", ses="baseline", output_dir=tmp_path)
182+
anat_q = ctx.bids(datatype="anat")
183+
tpl_q = anat_q.derive(ses="longitudinal")
184+
185+
resolved = resolve_longitudinal_anat(
186+
anat_q, tpl_q, anat_df, tpl_df, ses="baseline"
187+
)
188+
189+
assert resolved["brain_mask"] is None
190+
assert resolved["brain"] is not None
191+
192+
def test_xfm_query_uses_bids_safe_label(self, tmp_path: Path) -> None:
193+
"""Session labels with non-alphanumerics resolve through ``bids_safe_label``.
194+
195+
``pre-op`` is stored as ``from-preop`` in the xfm entity; the resolver
196+
must sanitize the ses arg before querying or the xfm won't be found.
197+
"""
198+
anat_df = _df(
199+
_anat_row(sub="01", ses="pre-op", suffix="T1w", desc="brain"),
200+
)
201+
tpl_df = _df(
202+
_anat_row(sub="01", ses="longitudinal", suffix="T1w", desc=None),
203+
_anat_row(
204+
sub="01",
205+
ses="longitudinal",
206+
suffix="xfm",
207+
desc=None,
208+
ext=".txt",
209+
extra=[
210+
{"key": "from", "value": "preop"},
211+
{"key": "to", "value": "longitudinal"},
212+
],
213+
),
214+
)
215+
216+
ctx = RunContext(sub="01", ses="pre-op", output_dir=tmp_path)
217+
anat_q = ctx.bids(datatype="anat")
218+
tpl_q = anat_q.derive(ses="longitudinal")
219+
220+
resolved = resolve_longitudinal_anat(
221+
anat_q, tpl_q, anat_df, tpl_df, ses="pre-op"
222+
)
223+
224+
assert resolved["subj_to_template_xfm"] is not None
225+
assert str(resolved["subj_to_template_xfm"]).endswith(
226+
"sub-01_ses-longitudinal_xfm.txt"
227+
)

tests/unit/orchestration/test_longitudinal.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from rbc.context import RunContext
1818
from rbc.orchestration import Filters
1919
from rbc.orchestration.longitudinal import process_anat, process_func
20+
from rbc.workflows.longitudinal.anatomical import AnatomicalLongOutputs
2021

2122
_SCHEMA = [
2223
"datatype",
@@ -48,17 +49,15 @@ def _func_row(sub: str, ses: str, task: str = "rest") -> tuple:
4849
return ("func", "bold", ".nii.gz", sub, ses, None, task, None, None, "/data", path)
4950

5051

51-
def _mock_anat_outputs() -> Mock:
52+
def _mock_anat_outputs() -> AnatomicalLongOutputs:
53+
"""Build a real AnatomicalLongOutputs so typos on missing attrs fail loudly."""
5254
fake = Path("fake_workdir")
53-
m = Mock()
54-
m.brain = fake / "brain.nii.gz"
55-
m.brain_mask = fake / "brain_mask.nii.gz"
56-
m.csf_mask = fake / "csf_mask.nii.gz"
57-
m.gm_mask = fake / "gm_mask.nii.gz"
58-
m.wm_mask = fake / "wm_mask.nii.gz"
59-
m.long_to_template_xfm = fake / "long_to_template_xfm.nii.gz"
60-
m.template_to_long_xfm = fake / "template_to_long_xfm.nii.gz"
61-
return m
55+
return AnatomicalLongOutputs(
56+
brain=fake / "brain.nii.gz",
57+
brain_mask=fake / "brain_mask.nii.gz",
58+
long_to_template_xfm=fake / "long_to_template_xfm.nii.gz",
59+
template_to_long_xfm=fake / "template_to_long_xfm.nii.gz",
60+
)
6261

6362

6463
def _mock_func_outputs(*, with_bold_mask: bool = True) -> Mock:
@@ -302,7 +301,7 @@ def test_missing_required_output_raises(
302301
pipe_ctx = RunContext(sub="01", ses="baseline", output_dir=tmp_path)
303302
outputs = _mock_anat_outputs()
304303
if side_effect is None:
305-
setattr(outputs, null_field, None)
304+
outputs = outputs._replace(**{null_field: None}) # type: ignore[arg-type]
306305
get_patch = patch(
307306
"rbc.bids.query.find_file",
308307
return_value=Path("fake_workdir/file.nii.gz"),

0 commit comments

Comments
 (0)