Skip to content

Integrate longitudinal template generation (Stage 2 of #301)#306

Merged
nx10 merged 4 commits into
mainfrom
refactor/longitudinal-stage-2
Apr 13, 2026
Merged

Integrate longitudinal template generation (Stage 2 of #301)#306
nx10 merged 4 commits into
mainfrom
refactor/longitudinal-stage-2

Conversation

@nx10
Copy link
Copy Markdown
Contributor

@nx10 nx10 commented Apr 13, 2026

Summary

Stage 2 of the longitudinal refactor (tracker: #301). Ports scripts/build_robust_template.py into the rbc package layout, updates xfm naming, and wires up rbc longitudinal template as a nested subcommand. The standalone script is deleted.

What moved / changed

  • Workflow split into proper layers
    • core/longitudinal/freesurfer.py -- generate_robust_template + fs_to_itk_xfm via niwrap freesurfer + rbc.core.fsl2itk (replaces c3d_affine_tool).
    • workflows/longitudinal/template.py -- LongitudinalTemplateOutputs + generate_subject_template.
    • bids/longitudinal/template.py -- discover_template_inputs + export_template.
    • orchestration/longitudinal/template.py -- per-subject loop.
  • Bug fix Implement BOLD Brain Masking #19 -- per-subject volume count check now lives inside the per-subject loop instead of on the whole df, so multi-subject runs no longer fail when one subject happens to have a single session.
  • xfm naming updated (Implement Slice Timing Correction #18) -- writes sub-X_ses-longitudinal_from-Y_to-longitudinal_mode-image_xfm.txt. resolve_longitudinal_{anat,func} now query by extra={"from": ses, "to": "longitudinal"}. The multi-entity-extras spike confirmed Bids.expect() ANDs each predicate, so this Just Works.
  • CLI -- cli/longitudinal/ is now a package with nested subparsers and a long alias. New: rbc longitudinal template. Legacy --anatomical/--functional flow lives at rbc longitudinal process until Stage 3 splits it into proper per-stage subcommands. (rbc long template works.)
  • No FS license required. Investigated upstream chklc(): if SURFER_SIDEDOOR is set, the check returns 1 immediately. Orchestration sets it on runner.environ, so users don't need --fs-license, FS_LICENSE, or any registration to run mri_robust_template. The original script's license-mounting plumbing is gone.
  • noit=True ported as-is with a TODO comment referencing Evaluate noit=True in longitudinal robust template generation #302 (quality spike).
  • Deletes scripts/build_robust_template.py.

Test plan

  • uv run pytest -m unit (750 passed, 1 skipped)
  • uv run ruff check src/ tests/
  • uv run ruff format src/ tests/
  • uv run mypy src/ tests/
  • uv run rbc longitudinal --help and rbc long template --help show the expected nested subparser output
  • Tier-2 integration test deferred to a follow-up PR. Acceptance criterion called for an integration test on the ds000114 fixture, but the multi-session fixture infrastructure (download script, CI cache key, conftest) is substantial enough to deserve its own PR. The existing pytest -m "integration and not slow" lane is unaffected (collects 0 tests today, so the "passes" criterion is trivially satisfied). I'll file a follow-up issue.

Follow-ups referenced

  • Evaluate noit=True in longitudinal robust template generation #302 -- noit=True quality spike (ported as-is here).
  • New issue to file: ds000114 fixture infrastructure + tier-2 integration tests for rbc longitudinal template.
  • Stage 3 (next PR) -- split rbc longitudinal process into dedicated anatomical/functional subcommands; remove the legacy --anatomical --functional flag shape.

Ports scripts/build_robust_template.py into the rbc package layout so the
template stage matches the rest of the pipeline and gets test coverage:

- core/longitudinal/freesurfer.py: generate_robust_template +
  fs_to_itk_xfm via niwrap freesurfer + rbc.core.fsl2itk (no c3d).
- workflows/longitudinal/template.py: LongitudinalTemplateOutputs +
  generate_subject_template().
- bids/longitudinal/template.py: discover_template_inputs +
  export_template; xfm output naming switched to
  sub-X_ses-longitudinal_from-Y_to-longitudinal_mode-image_xfm.txt.
  resolve_longitudinal_{anat,func} updated to query by from+to extras.
- orchestration/longitudinal/template.py: per-subject loop. Fixes #19
  (per-subject volume check now inside the loop, not on the whole df).
- cli/longitudinal/ becomes a package with nested subparsers and a
  ``long`` alias. ``rbc longitudinal template`` is the new subcommand;
  the legacy --anatomical/--functional flow lives at
  ``rbc longitudinal process`` until Stage 3 splits it.
- noit=True ported as-is with TODO referencing #302.
- Drops the FS license requirement entirely: orchestration sets the
  documented SURFER_SIDEDOOR bypass on the runner environ, since
  mri_robust_template's chklc() returns immediately when the var is set.
- Deletes scripts/build_robust_template.py.

Unit tests cover filename mapping, the per-subject single-volume guard,
template input discovery, BIDS export naming, and the new CLI wiring
(including the ``long`` alias).

Tier-2 integration test for ``rbc longitudinal template`` deferred:
needs the ds000114 multi-session fixture infrastructure (download
script + CI cache + conftest), which is substantial enough to land
in a separate PR. Existing ``pytest -m 'integration and not slow'``
collects 0 tests today, so no regression risk.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 13, 2026

Coverage

Coverage Report
FileStmtsMissCoverMissing
rbc
   __init__.py10100% 
   context.py25196%77
   metadata.py670100% 
rbc/bids
   __init__.py90100% 
   _schema.py585499%776, 782, 790, 1101
   anatomical.py24387%44, 47–48
   builder.py72790%233–235, 362, 364–365, 386
   functional.py42588%46–49, 84
   metrics.py23195%44
   qc.py170100% 
   query.py674237%103–107, 121–125, 127–128, 130–135, 137, 139, 153, 159–165, 198, 207, 209–216, 225, 257, 266–267
   session.py470100% 
rbc/bids/longitudinal
   __init__.py00100% 
   anatomical.py170100% 
   functional.py110100% 
   template.py270100% 
rbc/cli
   __init__.py10100% 
   all.py49295%84, 109
   anatomical.py25292%47, 63
   base.py71987%56, 62, 123–125, 131–133, 156
   functional.py33293%67, 87
   main.py420100% 
   metrics.py42295%77, 95
   qc.py25292%45, 61
rbc/cli/longitudinal
   __init__.py80100% 
   process.py27292%52, 69
   template.py230100% 
rbc/core
   __init__.py30100% 
   common.py261253%43–45, 62–70
   fileops.py27485%69–72
   fsl2itk.py420100% 
   nifti.py192597%236–237, 244–245, 524
   niwrap.py711381%59, 143–144, 146–148, 150–151, 156–157, 159, 161–162
rbc/core/anatomical
   __init__.py40100% 
   registration.py15473%59, 151, 166, 183
   segmentation.py24866%64, 74–76, 92, 114, 125, 141
rbc/core/functional
   __init__.py130100% 
   coregistration.py7271%44, 55
   despiking.py7357%32, 36–37
   distortion.py1304069%269–271, 321, 324, 332–335, 341–346, 349, 352–353, 356, 365–369, 375–376, 387–388, 391, 397, 443–444, 447, 455, 461, 470–471, 474, 484, 491
   erosion.py32196%50
   initialization.py9455%35, 42–43, 63
   masking.py342526%53, 55–56, 58–59, 62–65, 69, 91, 134, 183, 197, 208, 223, 233, 249, 258, 271, 285, 296, 306, 319, 328
   motion.py573735%62, 64–67, 69, 71, 73, 76–77, 83–84, 86–87, 95–97, 99, 102, 105, 107, 124–125, 135–138, 159, 169, 171–172, 175, 177–179, 181, 183
   nuisance.py816025%78, 80–85, 87, 89–90, 93, 96–98, 100–102, 104–110, 112–116, 118, 163, 165, 167, 170–171, 173–175, 178, 181–182, 185–187, 190–193, 197, 203–205, 207, 235, 243, 269, 278, 307, 316, 322
   regressors.py89693%163, 193, 325–328
   resampling.py544320%37–42, 74–76, 78, 80–81, 85–87, 90, 93, 105, 107–108, 112, 114, 150–152, 154–155, 159, 161–162, 167–169, 173–174, 177, 180, 187, 199, 201–202, 207, 209
   timing.py161131%46–47, 49–53, 58, 60, 66–67
rbc/core/longitudinal
   __init__.py10100% 
   freesurfer.py350100% 
   transform.py46784%106–107, 165–168, 170
rbc/core/metrics
   __init__.py30100% 
   alff.py90198%265
   reho.py660100% 
   smoothing.py7357%36, 42–43
   standardization.py271159%64, 66–68, 70, 72–75, 77–78
   timeseries.py70198%147
rbc/core/qc
   __init__.py60100% 
   dvars.py260100% 
   motion.py310100% 
   registration.py410100% 
   xcp.py410100% 
rbc/orchestration
   __init__.py32293%90, 93
   all.py41978%131–132, 137, 145, 153–154, 171, 173–174
   anatomical.py372240%49–52, 57–58, 60–63, 85–87, 89–90, 94, 101, 104, 109–110, 116, 118
   functional.py411270%124–126, 128–129, 133, 139, 142, 147–148, 157, 159
   metrics.py463034%32–35, 38–40, 67, 75–76, 100–102, 104–106, 110, 118–121, 123–124, 130–132, 137, 145, 152, 154
   qc.py340100% 
rbc/orchestration/longitudinal
   __init__.py38197%78
   anatomical.py160100% 
   functional.py140100% 
   template.py51590%48, 53–55, 101
rbc/workflows
   __init__.py100100% 
   anatomical.py291162%88–91, 95–100, 105
   functional.py925738%108–109, 117, 122–123, 131, 202–203, 206–207, 210–211, 214, 217–220, 230–232, 242–243, 246, 249, 252–254, 260, 263, 267–268, 275–276, 283–284, 291–292, 295–297, 300–303, 315–316, 328, 330–333, 336–337, 346–347, 355, 362
   metrics.py411856%80, 83–84, 93–94, 99–102, 105–108, 112–115, 119
   qc.py553438%88–89, 92–93, 96, 99, 102–107, 110, 119–121, 125–128, 130–134, 136–137, 139–141, 144, 161, 167, 169
rbc/workflows/longitudinal
   __init__.py00100% 
   anatomical.py24866%80–83, 85–87, 91
   functional.py11281%66, 72
   template.py15566%54–55, 59–60, 68
rbc_resources
   __init__.py380100% 
TOTAL346860182% 

Tests Skipped Failures Errors Time
757 0 💤 0 ❌ 0 🔥 11.191s ⏱️

nx10 added 3 commits April 13, 2026 15:15
- ``--fs-license`` / ``FS_LICENSE`` env var override the SURFER_SIDEDOOR
  bypass when set; license is mounted into containers via the restored
  mount_fs_license helper. Bypass remains the default so casual users
  don't need a license.
- ``discover_template_inputs`` now returns ``(inputs, skipped)`` and the
  orchestrator logs a warning per single-session subject instead of
  silently dropping them.
- ``hasattr(runner, "environ")`` guard around the SURFER_SIDEDOOR
  assignment for runners that don't expose an environ dict.
@nx10 nx10 merged commit eff2939 into main Apr 13, 2026
7 of 8 checks passed
@nx10 nx10 deleted the refactor/longitudinal-stage-2 branch April 13, 2026 19:33
nx10 added a commit that referenced this pull request Apr 13, 2026
Adds the download script, conftest fixtures, and CI cache that the
Stage 2 longitudinal integration test (deferred from PR #306) and the
upcoming Stages 3-6 tier-2 tests need. Pins OpenNeuro snapshot 1.0.2
via sha256 of task-fingerfootlips_bold.json so silent upstream drift
(including the old missing-TaskName regression) is caught at fetch
time. Also lands the deferred rbc longitudinal template integration
test asserting the ses-longitudinal BIDS tree shape (template T1w plus
from-test / from-retest ITK xfms).

Refs #301, follow-up to #306.
nx10 added a commit that referenced this pull request Apr 14, 2026
Adds the download script, conftest fixtures, and CI cache that the
Stage 2 longitudinal integration test (deferred from PR #306) and the
upcoming Stages 3-6 tier-2 tests need. Pins OpenNeuro snapshot 1.0.2
via sha256 of task-fingerfootlips_bold.json so silent upstream drift
(including the old missing-TaskName regression) is caught at fetch
time. Also lands the deferred rbc longitudinal template integration
test asserting the ses-longitudinal BIDS tree shape (template T1w plus
from-test / from-retest ITK xfms).

Refs #301, follow-up to #306.
nx10 added a commit that referenced this pull request Apr 14, 2026
The LTA that mri_robust_template writes embeds /styx_input/... paths to
the --mov volumes that only exist inside that container. When
lta_convert runs in a fresh container, those paths don't resolve and
FreeSurfer fails to emit a valid FSL matrix. Passing src_geometry and
trg_geometry explicitly makes niwrap bind-mount the volumes and gives
FreeSurfer the geometry it needs. Tightens the existing unit test to
assert both kwargs are forwarded so this can't regress silently again.

Latent since PR #306; surfaced by the ds000114 fixture in #307.
nx10 added a commit that referenced this pull request Apr 15, 2026
Adds the download script, conftest fixtures, and CI cache that the
Stage 2 longitudinal integration test (deferred from PR #306) and the
upcoming Stages 3-6 tier-2 tests need. Pins OpenNeuro snapshot 1.0.2
via sha256 of task-fingerfootlips_bold.json so silent upstream drift
(including the old missing-TaskName regression) is caught at fetch
time. Also lands the deferred rbc longitudinal template integration
test asserting the ses-longitudinal BIDS tree shape (template T1w plus
from-test / from-retest ITK xfms).

Refs #301, follow-up to #306.
nx10 added a commit that referenced this pull request Apr 15, 2026
The LTA that mri_robust_template writes embeds /styx_input/... paths to
the --mov volumes that only exist inside that container. When
lta_convert runs in a fresh container, those paths don't resolve and
FreeSurfer fails to emit a valid FSL matrix. Passing src_geometry and
trg_geometry explicitly makes niwrap bind-mount the volumes and gives
FreeSurfer the geometry it needs. Tightens the existing unit test to
assert both kwargs are forwarded so this can't regress silently again.

Latent since PR #306; surfaced by the ds000114 fixture in #307.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement BOLD Brain Masking

1 participant