ruff passes locally

Author: mattcieslak

.github/workflows/shellcheck.yml

@@ -16,7 +16,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Run shellcheck
       run: |
-        shellcheck \
+        shellcheck -x \
         tests/e2e-slurm/container/babs-user-script.sh \
         tests/e2e-slurm/container/ensure-env.sh \
         tests/e2e-slurm/container/walkthrough-tests.sh \

babs/constants.py

@@ -1,5 +1,5 @@
 MSG_NO_ALERT_IN_LOGS = 'BABS: No alert message found in log files.'
-CHECK_MARK = '\N{check mark}'  # can be used by print(CHECK_MARK)
+CHECK_MARK = '\N{CHECK MARK}'  # can be used by print(CHECK_MARK)
 PATH_FS_LICENSE_IN_CONTAINER = '/SGLR/FREESURFER_HOME/license.txt'
 
 # The upper layer of output folder - BABS expects there are sub-folers in it to zip:

pyproject.toml

@@ -147,10 +147,12 @@ inline-quotes = "single"
 [tool.ruff.lint.extend-per-file-ignores]
 "*/test_*.py" = ["S101"]
 "docs/conf.py" = ["A001"]
+"docs/source/conf.py" = ["A001"]
 "docs/sphinxext/github_link.py" = ["BLE001"]
 
 [tool.ruff.format]
 quote-style = "single"
+exclude = ["docs/source/conf.py"]
 
 [tool.pytest.ini_options]
 addopts = '-m "not integration"'

tests/test_babs_check_setup.py

@@ -32,7 +32,9 @@
 @pytest.mark.parametrize(
     'which_case', [('not_to_keep_failed'), ('wrong_container_ds'), ('wrong_input_ds')]
 )
-def test_babs_check_setup(which_case, tmp_path, tmp_path_factory, container_ds_path, if_circleci):
+def test_babs_check_setup(
+    which_case, tmp_path, tmp_path_factory, container_ds_path_param, if_circleci_param
+):
     """
     This is to test `babs-check-setup` in different failed `babs-init` cases.
     Successful `babs-init` has been tested in `test_babs_init.py`.
@@ -50,8 +52,10 @@ def test_babs_check_setup(which_case, tmp_path, tmp_path_factory, container_ds_p
         so expected error will be: BABS project does not exist.
     tmp_path: fixture from pytest
     tmp_path_factory: fixture from pytest
-    container_ds_path: fixture; str
+    container_ds_path_param: fixture; str
         Path to the container datalad dataset
+    if_circleci_param: fixture
+        CircleCI environment indicator
     """
     # fixed variables:
     which_bidsapp = 'toybidsapp'
@@ -66,8 +70,8 @@ def test_babs_check_setup(which_case, tmp_path, tmp_path_factory, container_ds_p
     input_ds_cli_wrong = [[which_input, '/random/path/to/input_ds']]
 
     # Container dataset - has been set up by fixture `prep_container_ds_toybidsapp()`
-    assert op.exists(container_ds_path)
-    assert op.exists(op.join(container_ds_path, '.datalad/config'))
+    assert op.exists(container_ds_path_param)
+    assert op.exists(op.join(container_ds_path_param, '.datalad/config'))
     container_ds_path_wrong = '/random/path/to/container_ds'
 
     # Preparation of env variable `TEMPLATEFLOW_HOME`:
@@ -94,7 +98,7 @@ def test_babs_check_setup(which_case, tmp_path, tmp_path_factory, container_ds_p
         project_name=project_name,
         input=input_ds_cli,
         list_sub_file=None,
-        container_ds=container_ds_path,
+        container_ds=container_ds_path_param,
         container_name=container_name,
         container_config_yaml_file=container_config_yaml_file,
         type_session=type_session,

tests/test_babs_init.py

@@ -9,11 +9,12 @@
 
 sys.path.append('..')
 sys.path.append('../babs')
-from babs.utils import (read_yaml)   # noqa
-from babs.cli import (    # noqa
+from babs.utils import read_yaml  # noqa
+from babs.cli import (  # noqa
     babs_init_main,
-    babs_check_setup_main)
-from get_data import (   # noqa
+    babs_check_setup_main,
+)
+from get_data import (  # noqa
     get_input_data,
     container_ds_path,
     where_now,
@@ -23,33 +24,43 @@
     INFO_2ND_INPUT_DATA,
     LIST_WHICH_BIDSAPP,
     TOYBIDSAPP_VERSION_DASH,
-    TEMPLATEFLOW_HOME
+    TEMPLATEFLOW_HOME,
 )
 
+
 @pytest.mark.order(index=1)
 @pytest.mark.parametrize(
-    'which_bidsapp,which_input,type_session,if_input_local,if_two_input',
+    ('which_bidsapp', 'which_input', 'type_session', 'if_input_local', 'if_two_input'),
     #  test toybidsapp: BIDS/zipped x single/multi-ses:
     #    the input data will also be remote by default:
-    [('toybidsapp', 'BIDS', 'single-ses', False, False),
-     ('toybidsapp', 'BIDS', 'multi-ses', False, False),
-     ('toybidsapp', 'fmriprep', 'single-ses', False, False),
-     ('toybidsapp', 'fmriprep', 'multi-ses', False, False),
-     # test if input is local:
-     ('toybidsapp', 'BIDS', 'single-ses', True, False),
-     # test fmriprep: single/multi-ses
-     ('fmriprep', 'BIDS', 'single-ses', False, False),
-     ('fmriprep', 'BIDS', 'multi-ses', False, False),
-     # test qsiprep multi-ses: remove sessions without dMRI
-     ('qsiprep', 'BIDS', 'multi-ses', False, False),
-     # test 2 input datasets (2nd one will be zipped fmriprep derivatives):
-     ('fmriprep', 'BIDS', 'single-ses', False, True),
-     ('fmriprep', 'BIDS', 'multi-ses', False, True),
-     ])
-def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_two_input,
-                   tmp_path, tmp_path_factory,
-                   container_ds_path, if_circleci,
-                   ):
+    [
+        ('toybidsapp', 'BIDS', 'single-ses', False, False),
+        ('toybidsapp', 'BIDS', 'multi-ses', False, False),
+        ('toybidsapp', 'fmriprep', 'single-ses', False, False),
+        ('toybidsapp', 'fmriprep', 'multi-ses', False, False),
+        # test if input is local:
+        ('toybidsapp', 'BIDS', 'single-ses', True, False),
+        # test fmriprep: single/multi-ses
+        ('fmriprep', 'BIDS', 'single-ses', False, False),
+        ('fmriprep', 'BIDS', 'multi-ses', False, False),
+        # test qsiprep multi-ses: remove sessions without dMRI
+        ('qsiprep', 'BIDS', 'multi-ses', False, False),
+        # test 2 input datasets (2nd one will be zipped fmriprep derivatives):
+        ('fmriprep', 'BIDS', 'single-ses', False, True),
+        ('fmriprep', 'BIDS', 'multi-ses', False, True),
+    ],
+)
+def test_babs_init(
+    which_bidsapp,
+    which_input,
+    type_session,
+    if_input_local,
+    if_two_input,
+    tmp_path,
+    tmp_path_factory,
+    container_ds_path_param,
+    if_circleci_param,
+):
     """
     This is to test `babs-init` in different cases.
 
@@ -70,9 +81,9 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
         whether to use two input datasets
     tmp_path: fixture from pytest
     tmp_path_factory: fixture from pytest
-    container_ds_path: fixture; str
+    container_ds_path_param: fixture; str
         Path to the container datalad dataset
-    if_circleci: fixture; bool
+    if_circleci_param: fixture; bool
         Whether currently in CircleCI
 
     TODO: add `type_system` and to test out Slurm version!
@@ -85,25 +96,27 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
     input_ds_cli = [[which_input, path_in]]
     if if_two_input:
         # get another input dataset: qsiprep derivatives
-        assert INFO_2ND_INPUT_DATA['which_input'] != which_input   # avoid repeated input ds name
+        assert INFO_2ND_INPUT_DATA['which_input'] != which_input  # avoid repeated input ds name
         path_in_2nd = get_input_data(
             INFO_2ND_INPUT_DATA['which_input'],
-            type_session,   # should be consistent with the 1st dataset
+            type_session,  # should be consistent with the 1st dataset
             INFO_2ND_INPUT_DATA['if_input_local'],
-            tmp_path_factory)
+            tmp_path_factory,
+        )
         input_ds_cli.append([INFO_2ND_INPUT_DATA['which_input'], path_in_2nd])
 
     # Container dataset - has been set up by fixture `prep_container_ds_toybidsapp()`
-    assert op.exists(container_ds_path)
-    assert op.exists(op.join(container_ds_path, '.datalad/config'))
+    assert op.exists(container_ds_path_param)
+    assert op.exists(op.join(container_ds_path_param, '.datalad/config'))
 
     # Preparation of freesurfer: for fmriprep and qsiprep:
     # check if `--fs-license-file` is included in YAML file:
-    container_config_yaml_filename = \
-        get_container_config_yaml_filename(which_bidsapp, which_input, if_two_input,
-                                           type_system='slurm')  # TODO: also test slurm!
-    container_config_yaml_file = op.join(op.dirname(__location__), 'notebooks',
-                                         container_config_yaml_filename)
+    container_config_yaml_filename = get_container_config_yaml_filename(
+        which_bidsapp, which_input, if_two_input, type_system='slurm'
+    )  # TODO: also test slurm!
+    container_config_yaml_file = op.join(
+        op.dirname(__location__), 'notebooks', container_config_yaml_filename
+    )
     assert op.exists(container_config_yaml_file)
     container_config_yaml = read_yaml(container_config_yaml_file)
 
@@ -117,11 +130,11 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
 
     # Preparation of env variable `TEMPLATEFLOW_HOME`:
     os.environ['TEMPLATEFLOW_HOME'] = TEMPLATEFLOW_HOME
-    assert os.getenv('TEMPLATEFLOW_HOME') is not None    # assert env var has been set
+    assert os.getenv('TEMPLATEFLOW_HOME') is not None  # assert env var has been set
     # as env var has been set up, expect that BABS will generate necessary cmd for templateflow
 
     # Get the cli of `babs-init`:
-    where_project = tmp_path.absolute().as_posix()   # turn into a string
+    where_project = tmp_path.absolute().as_posix()  # turn into a string
     project_name = 'my_babs_project'
     project_root = op.join(where_project, project_name)
     container_name = which_bidsapp + '-' + TOYBIDSAPP_VERSION_DASH
@@ -131,28 +144,25 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
         project_name=project_name,
         input=input_ds_cli,
         list_sub_file=None,
-        container_ds=container_ds_path,
+        container_ds=container_ds_path_param,
         container_name=container_name,
         container_config_yaml_file=container_config_yaml_file,
         type_session=type_session,
         type_system='slurm',
-        keep_if_failed=False
+        keep_if_failed=False,
     )
 
     # run `babs-init`:
-    with mock.patch.object(
-            argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts):
+    with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts):
         babs_init_main()
 
     # ================== ASSERT ============================
     # Assert by running `babs-check-setup`
-    babs_check_setup_opts = argparse.Namespace(
-        project_root=project_root,
-        job_test=False
-    )
+    babs_check_setup_opts = argparse.Namespace(project_root=project_root, job_test=False)
     # Run `babs-check-setup`:
     with mock.patch.object(
-            argparse.ArgumentParser, 'parse_args', return_value=babs_check_setup_opts):
+        argparse.ArgumentParser, 'parse_args', return_value=babs_check_setup_opts
+    ):
         babs_check_setup_main()
 
     # Check if necessary commands in ``<container_name>-0-0-0_zip.sh`:
@@ -169,11 +179,10 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
     lines_bash_container_zip = file_bash_container_zip.readlines()
     file_bash_container_zip.close()
     # check:
-    if_bind_templateflow = False   # `singularity run -B` to bind a path to container
+    if_bind_templateflow = False  # `singularity run -B` to bind a path to container
     if_bind_freesurfer = False
-    str_bind_freesurfer = '-B ' + str_fs_license_file \
-        + ':/SGLR/FREESURFER_HOME/license.txt'
-    print(str_bind_freesurfer)   # FOR DEBUGGING
+    str_bind_freesurfer = '-B ' + str_fs_license_file + ':/SGLR/FREESURFER_HOME/license.txt'
+    print(str_bind_freesurfer)  # FOR DEBUGGING
 
     if_set_singu_templateflow = False  # `singularity run --env` to set env var within container
     if_generate_bidsfilterfile = False
@@ -183,8 +192,7 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
     for line in lines_bash_container_zip:
         if '--env TEMPLATEFLOW_HOME=/SGLR/TEMPLATEFLOW_HOME' in line:
             if_set_singu_templateflow = True
-        if all(ele in line for ele in ['-B',
-                                       TEMPLATEFLOW_HOME + ':/SGLR/TEMPLATEFLOW_HOME']):
+        if all(ele in line for ele in ['-B', TEMPLATEFLOW_HOME + ':/SGLR/TEMPLATEFLOW_HOME']):
             # e.g., `-B /test/templateflow_home:/SGLR/TEMPLATEFLOW_HOME \`
             if_bind_templateflow = True
         if str_bind_freesurfer in line:
@@ -197,40 +205,65 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
             if_flag_fs_license = True
     # assert they are found:
     # 1) TemplateFlow: should be found in all cases:
-    assert if_bind_templateflow, \
-        "Env variable 'TEMPLATEFLOW_HOME' has been set," \
-        + " but Templateflow home path did not get bound in 'singularity run'" \
-        + " with `-B` in '" + container_name + "_zip.sh'."
-    assert if_set_singu_templateflow, \
-        "Env variable 'TEMPLATEFLOW_HOME' has been set," \
-        + " but env variable 'SINGULARITYENV_TEMPLATEFLOW_HOME' was not set" \
-        + " with `--env` in '" + container_name + "_zip.sh'."
+    assert if_bind_templateflow, (
+        "Env variable 'TEMPLATEFLOW_HOME' has been set,"
+        " but Templateflow home path did not get bound in 'singularity run'"
+        " with `-B` in '" + container_name + "_zip.sh'."
+    )
+    assert if_set_singu_templateflow, (
+        "Env variable 'TEMPLATEFLOW_HOME' has been set,"
+        " but env variable 'SINGULARITYENV_TEMPLATEFLOW_HOME' was not set"
+        " with `--env` in '" + container_name + "_zip.sh'."
+    )
     # 2) BIDS filter file: only when qsiprep/fmriprep & multi-ses:
     if (which_bidsapp in ['qsiprep', 'fmriprep']) & (type_session == 'multi-ses'):
-        assert if_generate_bidsfilterfile, \
-            "This is BIDS App '" + which_bidsapp + "' and " + type_session + ',' \
-            + ' however, filterfile to be used in `--bids-filter-file` was not generated' \
-            + " in '" + container_name + "_zip.sh'."
-        assert if_flag_bidsfilterfile, \
-            "This is BIDS App '" + which_bidsapp + "' and " + type_session + ',' \
-            + ' however, flag `--bids-filter-file` was not included in `singularity run`' \
-            + " in '" + container_name + "_zip.sh'."
+        assert if_generate_bidsfilterfile, (
+            "This is BIDS App '"
+            + which_bidsapp
+            + "' and "
+            + type_session
+            + ','
+            + ' however, filterfile to be used in `--bids-filter-file` was not generated'
+            + " in '"
+            + container_name
+            + "_zip.sh'."
+        )
+        assert if_flag_bidsfilterfile, (
+            "This is BIDS App '"
+            + which_bidsapp
+            + "' and "
+            + type_session
+            + ','
+            + ' however, flag `--bids-filter-file` was not included in `singularity run`'
+            + " in '"
+            + container_name
+            + "_zip.sh'."
+        )
     else:
-        assert (not if_generate_bidsfilterfile) & (not if_flag_bidsfilterfile), \
-            "This is BIDS App '" + which_bidsapp + "' and " + type_session + ',' \
-            + ' so `--bids-filter-file` should not be generated or used in `singularity run`' \
-            + " in '" + container_name + "_zip.sh'."
+        assert (not if_generate_bidsfilterfile) & (not if_flag_bidsfilterfile), (
+            "This is BIDS App '"
+            + which_bidsapp
+            + "' and "
+            + type_session
+            + ','
+            + ' so `--bids-filter-file` should not be generated or used in `singularity run`'
+            + " in '"
+            + container_name
+            + "_zip.sh'."
+        )
     # 3) freesurfer license:
     if flag_requested_fs_license:
-        assert if_bind_freesurfer, \
-            "`--fs-license-file` was requested in container's YAML file," \
-            + " but FreeSurfer license path did not get bound in 'singularity run'" \
-            + " with `-B` in '" + container_name + "_zip.sh'."
-        assert if_flag_fs_license, \
-            "`--fs-license-file` was requested in container's YAML file," \
-            + ' but flag `' + flag_fs_license + '` was not found in the `singularity run`' \
-            + " in '" + container_name + "_zip.sh'." \
-            + " Path to YAML file: '" + container_config_yaml_file + "'."
+        assert if_bind_freesurfer, (
+            "`--fs-license-file` was requested in container's YAML file,"
+            " but FreeSurfer license path did not get bound in 'singularity run'"
+            " with `-B` in '" + container_name + "_zip.sh'."
+        )
+        assert if_flag_fs_license, (
+            "`--fs-license-file` was requested in container's YAML file,"
+            ' but flag `' + flag_fs_license + '` was not found in the `singularity run`'
+            " in '" + container_name + "_zip.sh'."
+            " Path to YAML file: '" + container_config_yaml_file + "'."
+        )
 
     # Check `sub_ses_final_inclu.csv`:
     #   if qsiprep + multi-ses:  one session without dMRI should not be included
@@ -244,9 +277,12 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
             if_inclu_missing_session = False
             if 'sub-02,ses-A' in line:
                 if_inclu_missing_session = True
-        assert not if_inclu_missing_session, \
-            "'sub-02,ses-A' without dMRI was included in the BABS project of " \
-            + which_bidsapp + ', ' + type_session
+        assert not if_inclu_missing_session, (
+            "'sub-02,ses-A' without dMRI was included in the BABS project of "
+            + which_bidsapp
+            + ', '
+            + type_session
+        )
 
     # Note: No need to manually remove temporary dirs; those are created by pytest's fixtures
     #   and will be automatically removed after 3 runs of pytests. ref below: