diff --git a/babs/templates/bidsapp_pipeline_run.sh.jinja2 b/babs/templates/bidsapp_pipeline_run.sh.jinja2 index 90198b20..729b2b89 100644 --- a/babs/templates/bidsapp_pipeline_run.sh.jinja2 +++ b/babs/templates/bidsapp_pipeline_run.sh.jinja2 @@ -15,7 +15,10 @@ sesid="$2" filterfile="${PWD}/${sesid}_filter.json" {% raw %}{{% endraw %} echo "{" - echo "'fmap': {'datatype': 'fmap', 'session': '$sesid'}," + {# The fmap entry must NOT carry 'session'. fmriprep passes this entry to sdcflows' + find_estimators() together with the session it resolved itself, and sdcflows rejects + receiving the session twice. The session is already enforced by the sparse checkout. #} + echo "'fmap': {'datatype': 'fmap'}," {% set first_filter_step = steps_with_filter[0] %} {% if 'fmriprep' in first_filter_step['container_name'].lower() %} echo "'bold': {'datatype': 'func', 'session': '$sesid', 'suffix': 'bold'}," diff --git a/babs/templates/bidsapp_run.sh.jinja2 b/babs/templates/bidsapp_run.sh.jinja2 index 12b5da0e..4e59f586 100644 --- a/babs/templates/bidsapp_run.sh.jinja2 +++ b/babs/templates/bidsapp_run.sh.jinja2 @@ -14,7 +14,10 @@ sesid="$2" filterfile="${PWD}/${sesid}_filter.json" {% raw %}{{% endraw %} echo "{" - echo "'fmap': {'datatype': 'fmap', 'session': '$sesid'}," + {# The fmap entry must NOT carry 'session'. fmriprep passes this entry to sdcflows' + find_estimators() together with the session it resolved itself, and sdcflows rejects + receiving the session twice. The session is already enforced by the sparse checkout. #} + echo "'fmap': {'datatype': 'fmap'}," {% if 'fmriprep' in container_name.lower() %} echo "'bold': {'datatype': 'func', 'session': '$sesid', 'suffix': 'bold'}," {% elif 'qsiprep' in container_name.lower() %} diff --git a/babs/templates/filter_file.sh.jinja2 b/babs/templates/filter_file.sh.jinja2 deleted file mode 100644 index 3898a7e7..00000000 --- a/babs/templates/filter_file.sh.jinja2 +++ /dev/null @@ -1,22 +0,0 @@ -filterfile=${PWD}/${sesid}_filter.json -echo "{" > ${filterfile} - -echo "'fmap': {'datatype': 'fmap'}," >> ${filterfile} - -{% if 'fmriprep' in container_name.lower() %} -echo "'bold': {'datatype': 'func', 'session': '$sesid', 'suffix': 'bold'}," >> ${filterfile} -{% elif 'qsiprep' in container_name.lower() %} -echo "'dwi': {'datatype': 'dwi', 'session': '$sesid', 'suffix': 'dwi'}," >> ${filterfile} -{% elif 'aslprep' in container_name.lower() %} -echo "'perf': {'datatype': 'perf', 'session': '$sesid', 'suffix': 'asl'}," >> ${filterfile} -{% endif %} -echo "'sbref': {'datatype': 'func', 'session': '$sesid', 'suffix': 'sbref'}," >> ${filterfile} -echo "'flair': {'datatype': 'anat', 'session': '$sesid', 'suffix': 'FLAIR'}," >> ${filterfile} -echo "'t2w': {'datatype': 'anat', 'session': '$sesid', 'suffix': 'T2w'}," >> ${filterfile} -echo "'t1w': {'datatype': 'anat', 'session': '$sesid', 'suffix': 'T1w'}," >> ${filterfile} -echo "'roi': {'datatype': 'anat', 'session': '$sesid', 'suffix': 'roi'}" >> ${filterfile} -echo "}" >> ${filterfile} - -# remove ses and get valid json -sed -i "s/'/\\\"/g" ${filterfile} -sed -i "s/ses-//g" ${filterfile} \ No newline at end of file diff --git a/tests/test_generate_bidsapp_runscript.py b/tests/test_generate_bidsapp_runscript.py index 19a34402..4f10343e 100644 --- a/tests/test_generate_bidsapp_runscript.py +++ b/tests/test_generate_bidsapp_runscript.py @@ -1,3 +1,5 @@ +import json +import os import subprocess from pathlib import Path @@ -139,6 +141,67 @@ def test_generate_bidsapp_runscript(input_datasets, config_file, processing_leve assert passed, status +def generate_session_filter_file(config_file, input_datasets, tmp_path): + """Render a session-level runscript, run its filter-file block, and parse the result. + + The filter file is written by the job script at runtime, so the only way to see what + a BIDS app actually receives is to execute the block that builds it. + """ + config = read_yaml(NOTEBOOKS_DIR / config_file) + _, bids_app_output_dir = app_output_settings_from_config(config) + script_content = generate_bidsapp_runscript( + input_datasets, + 'session', + container_name=config_file.split('_')[1], + relative_container_path='containers/.datalad/containers/app/image', + bids_app_output_dir=bids_app_output_dir, + dict_zip_foldernames=config['zip_foldernames'], + bids_app_args=config['bids_app_args'], + singularity_args=config['singularity_args'], + templateflow_home='/path/to/templateflow_home', + ) + + # the block runs from `filterfile=...` through the last `sed -i` that repairs the JSON + lines = script_content.splitlines() + start = next(i for i, line in enumerate(lines) if line.startswith('filterfile=')) + end = max(i for i, line in enumerate(lines) if line.startswith('sed -i')) + block = '\n'.join(lines[start : end + 1]) + + subprocess.run( + ['bash', '-c', block], + cwd=tmp_path, + env={'sesid': 'ses-1', 'PATH': os.environ['PATH']}, + check=True, + ) + return json.loads((tmp_path / 'ses-1_filter.json').read_text()) + + +@pytest.mark.parametrize( + 'config_file', + [ + 'eg_fmriprep-24-1-1_regular.yaml', + 'eg_qsiprep-1-0-0_regular.yaml', + 'eg_aslprep-0-7-5.yaml', + ], +) +def test_session_filter_file_fmap_carries_no_session(config_file, tmp_path): + """The fmap entry must not pin a session. + + fmriprep passes this entry straight to sdcflows' ``find_estimators()`` alongside the + session it resolved itself, and sdcflows raises "Filters include session, but session + is already defined." when it receives both. That aborts workflow construction, so no + session-level job can run. The session is already enforced by the sparse checkout. + """ + filters = generate_session_filter_file(config_file, input_datasets_prep, tmp_path) + + assert filters['fmap'] == {'datatype': 'fmap'} + + # the remaining entries still restrict to this session ('ses-' is stripped by sed) + others = {key: value for key, value in filters.items() if key != 'fmap'} + assert others + assert all(value.get('session') == '1' for value in others.values()) + + def run_shellcheck(script_path): """Run shellcheck on a shell script string and return the result.