Fix SBRef sorting when EchoTime is missing

fmriprep/workflows/bold/fit.py

@@ -7,7 +7,7 @@
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
-#     http://www.apache.org/licenses/LICENSE-2.0
+#     https://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
@@ -20,6 +20,7 @@
 #
 #     https://www.nipreps.org/community/licensing/
 #
+import logging
 import os
 import re
 import typing as ty
@@ -86,10 +87,21 @@ def get_sbrefs(
     entities.update(suffix='sbref', extension=['.nii', '.nii.gz'])
     entities.update(entity_overrides)
 
-    return sorted(
-        layout.get(return_type='file', **entities),
-        key=lambda fname: layout.get_metadata(fname).get('EchoTime'),
-    )
+    sbref_files = layout.get(return_type='file', **entities)
+    if len(sbref_files) == 1:
+        return sbref_files
+
+    valid_sbref_files = []
+
+    for fname in sbref_files:
+        if (echo_time := layout.get_metadata(fname).get('EchoTime')) is None:
+            logging.getLogger('nipype.workflow').warning(
+                'Dropping SBRef without EchoTime metadata: %s', fname
+            )
+            continue
+        valid_sbref_files.append((echo_time, fname))
+
+    return [fname for _, fname in sorted(valid_sbref_files)]
 
 
 def init_bold_fit_wf(
@@ -620,8 +632,7 @@ def init_bold_fit_wf(
                     ('out_file', 'inputnode.boldref'),
                 ]),
                 (ds_coreg_boldref_wf, skullstrip_bold_wf, [
-                    ('outputnode.boldref', 'inputnode.in_file'),
-                ]),
+                    ('outputnode.boldref', 'inputnode.in_file')]),
                 (skullstrip_bold_wf, ds_boldmask_wf, [
                     ('outputnode.mask_file', 'inputnode.boldmask'),
                 ]),

fmriprep/workflows/bold/tests/test_fit.py

@@ -1,3 +1,4 @@
+import logging
 from pathlib import Path
 
 import nibabel as nb
@@ -9,7 +10,7 @@
 from .... import config
 from ...tests import mock_config
 from ...tests.layouts import get_layout
-from ..fit import init_bold_fit_wf, init_bold_native_wf
+from ..fit import get_sbrefs, init_bold_fit_wf, init_bold_native_wf
 
 
 @pytest.fixture(scope='module', autouse=True)
@@ -47,6 +48,52 @@ def _make_params(
     )
 
 
+def test_get_sbrefs_rejects_missing_echo_time(caplog):
+    """SBRefs without EchoTime metadata should be dropped with a warning."""
+    bold_files = [
+        '/bids/sub-01/func/sub-01_task-rest_run-01_echo-1_bold.nii.gz',
+        '/bids/sub-01/func/sub-01_task-rest_run-01_echo-2_bold.nii.gz',
+    ]
+    sbref_files = [
+        '/bids/sub-01/func/sub-01_task-rest_run-01_echo-2_sbref.nii.gz',
+        '/bids/sub-01/func/sub-01_task-rest_run-01_echo-1_sbref.nii.gz',
+    ]
+
+    class Layout:
+        def get(self, **_entities):
+            return list(sbref_files)
+
+        def get_metadata(self, fname):
+            return {'EchoTime': 0.01} if fname.endswith('echo-1_sbref.nii.gz') else {}
+
+    logger = logging.getLogger('nipype.workflow')
+    old_propagate = logger.propagate
+    logger.propagate = True
+    with caplog.at_level(logging.WARNING, logger='nipype.workflow'):
+        found = get_sbrefs(bold_files, {}, Layout())
+    logger.propagate = old_propagate
+
+    assert found == ['/bids/sub-01/func/sub-01_task-rest_run-01_echo-1_sbref.nii.gz']
+    assert 'Dropping SBRef without EchoTime metadata' in caplog.text
+
+
+def test_get_sbrefs_preserves_single_missing_echo_time():
+    """A single SBRef without EchoTime should still be returned."""
+    bold_files = ['/bids/sub-01/func/sub-01_task-rest_run-01_bold.nii.gz']
+    sbref_file = '/bids/sub-01/func/sub-01_task-rest_run-01_sbref.nii.gz'
+
+    class Layout:
+        def get(self, **_entities):
+            return [sbref_file]
+
+        def get_metadata(self, _fname):
+            return {}
+
+    found = get_sbrefs(bold_files, {}, Layout())
+
+    assert found == [sbref_file]
+
+
 @pytest.mark.parametrize('task', ['rest', 'nback'])
 @pytest.mark.parametrize('fieldmap_id', ['phasediff', None])
 @pytest.mark.parametrize(