Apply suggestions from code review

Co-authored-by: Chris Markiewicz <[email protected]>
nipreps · Sep 27, 2022 · 2052f3e · 2052f3e
1 parent 2dce9b2
commit 2052f3e
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 25 deletions.
diff --git a/fmriprep/cli/tests/test_parser.py b/fmriprep/cli/tests/test_parser.py
@@ -156,9 +156,9 @@ def test_parse_args(tmp_path):
             "participant",  # BIDS App
             "-w",
             str(work_dir),  # Don't pollute CWD
-            "--skip-bids-validation",
+            "--skip-bids-validation",  # Empty files make BIDS sad
         ]
-    )  # Empty files make BIDS sad
+    )
     assert config.execution.layout.root == bids_dir
     _reset_config()
 

diff --git a/fmriprep/interfaces/confounds.py b/fmriprep/interfaces/confounds.py
@@ -171,11 +171,10 @@ def _run_interface(self, runtime):
         a_orig = a_comp_cor["component"]
         a_new = [f"a_comp_cor_{i:02d}" for i in range(len(a_orig))]
 
-        (
-            components.rename(columns=dict(zip(c_orig, c_new)))
-            .rename(columns=dict(zip(w_orig, w_new)))
-            .rename(columns=dict(zip(a_orig, a_new)))
-        ).to_csv(self._results["components_file"], sep='\t', index=False)
+    final_components = components.rename(columns=dict(zip(c_orig, c_new)))
+    final_components.rename(columns=dict(zip(w_orig, w_new)), inplace=True)
+    final_components.rename(columns=dict(zip(a_orig, a_new)), inplace=True)
+    final_components.to_csv(self._results["components_file"], sep='\t', index=False)
 
         metadata.loc[c_comp_cor.index, "component"] = c_new
         metadata.loc[w_comp_cor.index, "component"] = w_new

diff --git a/fmriprep/interfaces/multiecho.py b/fmriprep/interfaces/multiecho.py
@@ -68,11 +68,9 @@ class T2SMapInputSpec(CommandLineInputSpec):
         usedefault=True,
         desc=(
             'Desired fitting method: '
-            '"loglin" means that a linear model is fit '
-            'to the log of the data. '
-            '"curvefit" means that a more computationally '
-            'demanding monoexponential model is fit '
-            'to the raw data.'
+            '"loglin" means that a linear model is fit to the log of the data. '
+            '"curvefit" means that a more computationally demanding '
+            'monoexponential model is fit to the raw data.'
         ),
     )
 

diff --git a/fmriprep/interfaces/reports.py b/fmriprep/interfaces/reports.py
@@ -238,9 +238,11 @@ class FunctionalSummary(SummaryInterface):
 
     def _generate_segment(self):
         dof = self.inputs.registration_dof
-        stc = {True: 'Applied', False: 'Not applied', 'TooShort': 'Skipped (too few volumes)'}[
-            self.inputs.slice_timing
-        ]
+        stc = {
+            True: 'Applied',
+            False: 'Not applied',
+            'TooShort': 'Skipped (too few volumes)',
+        }[self.inputs.slice_timing]
         # #TODO: Add a note about registration_init below?
         reg = {
             'FSL': [

diff --git a/fmriprep/reports/core.py b/fmriprep/reports/core.py
@@ -122,7 +122,7 @@ def generate_reports(
 
         logger = logging.getLogger("cli")
         error_list = ", ".join(
-            "%s (%d)" % (subid, err) for subid, err in zip(subject_list, report_errors) if err
+            f"{subid} ({err})" for subid, err in zip(subject_list, report_errors) if err
         )
         logger.error(
             "Preprocessing did not finish successfully. Errors occurred while processing "

diff --git a/fmriprep/workflows/base.py b/fmriprep/workflows/base.py
@@ -373,17 +373,16 @@ def init_single_subject_wf(subject_id):
 
         if config.workflow.use_syn_sdc and not fmap_estimators:
             message = (
-                "Fieldmap-less (SyN) estimation was requested, but "
-                "PhaseEncodingDirection information appears to be "
-                "absent."
+                "Fieldmap-less (SyN) estimation was requested, but PhaseEncodingDirection "
+                "information appears to be absent."
             )
             config.loggers.workflow.error(message)
             if config.workflow.use_syn_sdc == "error":
                 raise ValueError(message)
 
-        if "fieldmaps" in config.workflow.ignore and [
-            f for f in fmap_estimators if f.method != fm.EstimatorType.ANAT
-        ]:
+        if "fieldmaps" in config.workflow.ignore and any(
+            f.method == fm.EstimatorType.ANAT for f in fmap_estimators
+        ):
             config.loggers.workflow.info(
                 'Option "--ignore fieldmaps" was set, but either "--use-syn-sdc" '
                 'or "--force-syn" were given, so fieldmap-less estimation will be executed.'

diff --git a/fmriprep/workflows/bold/base.py b/fmriprep/workflows/bold/base.py
@@ -202,9 +202,8 @@ def init_func_preproc_wf(bold_file, has_fieldmap=False):
     )
     from niworkflows.interfaces.utility import DictMerge, KeySelect
 
-    if nb.load(bold_file[0] if isinstance(bold_file, (list, tuple)) else bold_file).shape[3:] <= (
-        5 - config.execution.sloppy,
-    ):
+    nvols = nb.load(bold_file[0] if isinstance(bold_file, (list, tuple)) else bold_file).shape[3]
+    if nvols <= 5 - config.execution.sloppy:
         config.loggers.workflow.warning(
             f"Too short BOLD series (<= 5 timepoints). Skipping processing of <{bold_file}>."
         )