Fix session filter dropping sessionless datasets; add rbc-all integration test#290
Merged
Conversation
9dce206 to
c94b0aa
Compare
…gration test (#289) `pl.col("ses") != "longitudinal"` evaluates to null (falsy) when ses is None, silently skipping every row in datasets without session labels. Replace with `ne_missing()` in all three cross-sectional orchestration modules so null sessions are preserved. Add an integration test that runs `rbc all` on tests/data/ds000001 with Docker and verifies key derivative files are produced.
…al stages Tests both invocation styles (rbc all vs anatomical/functional/metrics/qc in sequence) and verifies they produce the same set of derivative files. Uses session-scoped fixtures so each pipeline variant runs only once.
The functional/metrics/qc stages need to discover derivatives from previous stages via load_table, so the output dir must be included in the input dirs alongside the raw BIDS dataset.
resolve_metrics and resolve_qc queried extra entities with raw regressor names (e.g. "36-parameter") but export_functional saves them through bids_safe_label (e.g. "36parameter"). This mismatch caused FileNotFoundError when running rbc metrics or rbc qc standalone.
Copies ds000001 and truncates the BOLD timeseries before running the pipeline, cutting functional processing time significantly while still exercising the full code path (motion correction, nuisance regression, metrics, QC).
25 volumes was too few: after discarding 2 TRs, only 23 timepoints remained, which is fewer than the ~36 columns in the 36-parameter nuisance regressor model, causing 3dTproject to fail.
Remove the volume truncation optimization so we can shake out all bugs with the real data first. Can revisit truncation once the pipeline passes end-to-end.
The sequential test now copies anat derivatives from the rbc-all run instead of re-running brain extraction and registration, saving ~30-40 min of CI time. The disk round-trip handoff between functional, metrics, and QC stages is still fully tested.
f793cc7 to
4404b9a
Compare
The BIDS path builder formats integer run entities as "run-1" (not "run-01"), so update the test assertion accordingly. resolve_qc's template_bold query matched both the untagged preproc bold and per-regressor preproc bolds. Add without=["reg"] to disambiguate.
Use extra={"reg": False} instead of without=["reg"] since reg is an
extra entity (not a standard BIDS column). Also relax the QC glob
pattern since entity ordering puts reg before desc in the filename.
compute_alff, compute_reho, and compute_timeseries default to writing outputs next to their input files. When the standalone metrics/QC orchestration resolves inputs from the derivatives dir, these intermediates leak into the output alongside the BIDS-named exports. Use generate_exec_folder() to create a dedicated work directory under the niwrap runner's data_dir, and pass explicit out_file/out_dir paths so intermediates stay out of the derivatives tree.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #289
Summary
Integration test that runs
rbc alland the four individual subcommands (anatomical/functional/metrics/qc) ontests/data/ds000001, then verifies both produce the expected derivative files and match each other.Writing this test exposed several pre-existing bugs:
pl.col("ses") != "longitudinal"evaluates to null (falsy) whensesis None. Fixed withne_missing()in all three cross-sectional orchestration modules.resolve_metrics/resolve_qclabel mismatch: Resolve functions queried raw regressor names (e.g."36-parameter") but exports usebids_safe_label()(e.g."36parameter"). Fixed by applyingbids_safe_label()in both resolve functions.resolve_qcduplicate match ontemplate_bold: Query matched both the untagged preproc bold and per-regressor preproc bolds. Fixed withextra={"reg": False}to exclude files with aregentity.Also pins
uvversion in the full test workflow to work around araw.githubusercontent.comnetwork block on the self-hosted runner.Test plan
uv run pytest tests/integration/test_all.py -m slow --runner dockerpasses end-to-enduv run pytest -m unit)