Moving Fastsurfer and Pytest environment setup to local_runner image,…

… thus simplifying, and streamlining the workflow and improving execution time significantly.

Adding Labelled output directories for each run

Removing unnecessary import statements from pytest files

Cleaning up code:

- migrating from os.path.join to pathlib Path

- changing str to Path

- cleaning up comments and docstrings

- ran ruff on code to fix code style

.github/workflows/quicktest_runner.yaml

@@ -62,53 +62,11 @@ jobs:
           echo "Error: FreeSurfer installation directory does not exist at $FREESURFER_HOME"
           exit 1
         fi
-      # Set up Python using setup-python@v3 action
-    - name: Set up Python 3.10
-      uses: actions/setup-python@v3
-      with:
-        python-version: "3.10"
-      # Check if FastSurfer environment already exists
-    - name: Check Environment
-      run: |
-        if [ ! -f "$MAMBAPATH" ]; then
-          echo "FastSurfer environment does not exist. Creating FastSurfer Environment..."
-          echo "HAS_MAMBA=true" >> $GITHUB_OUTPUT
-        else
-          echo "FastSurfer environment exists. Skipping FastSurfer Environment creation..."
-          echo "HAS_MAMBA=false" >> $GITHUB_OUTPUT
-        fi
-      id: check-environment
-      # Create FastSurfer environment if it does not exist
-    - name: Create FastSurfer Environment
-      uses: mamba-org/setup-micromamba@v1
-      with:
-        environment-file: env/fastsurfer.yml
-        micromamba-root-path: $MAMBAROOT
-        micromamba-binary-path: $MAMBAPATH
-        cache-downloads: true
-        cache-environment: true
-        init-shell: none
-      if: steps.check-environment.outputs.HAS_MAMBA == 'true'
-      # Set up cache for python packages
-    - name: Cache Python packages
-      uses: actions/cache@v3
-      with:
-        path: /home/.cache/pip
-        key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
-        restore-keys: |
-          ${{ runner.os }}-pip-
-      # Install required python packages
-    - name: Install package
-      run: |
-        python -m pip install --progress-bar off --upgrade pip \
-        setuptools wheel
-        python -m pip install --progress-bar off .[test]
-      # Run fastsurfer on list of subjects
+    # Run FastSurfer on test subjects
     - name: Run FastSurfer
       run: |
         echo "Running FastSurfer..."
-        export FREESURFER_HOME=/freesurfer
-        source $FREESURFER_HOME/SetUpFreeSurfer.sh
+        echo "Output will be saved in data/${GITHUB_SHA:0:7}"
         export FASTSURFER_HOME=$(pwd)
         export THIS_RUN_OUTDIR=${GITHUB_SHA:0:7}
         mkdir -p $SUBJECTS_DIR/$THIS_RUN_OUTDIR
@@ -120,18 +78,10 @@ jobs:
   # Test fastsurfer output
   run-pytest:
     runs-on: self-hosted
+    if: always()
     needs: run-fastsurfer
     steps:
-    - name: Set up Python 3.10
-      uses: actions/setup-python@v3
-      with:
-        python-version: "3.10"
-    - name: Cache Python packages
-      uses: actions/cache@v3
-      with:
-        path: /home/.cache/pip
-        key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
-        restore-keys: |
-          ${{ runner.os }}-pip-
     - name: Run pytest
-      run: python -m pytest test/quick_test
+      run: |
+        source /venv-pytest/bin/activate
+        python -m pytest test/quick_test

test/__init__.py

@@ -1,8 +1,5 @@
-
-
-
-__all__ = [ # This is a list of modules that should be imported when using the import * syntax
-    'test_file_existence',
-    'test_error_messages',
-    'test_errors'
-    ]  
+__all__ = [  # This is a list of modules that should be imported when using the import * syntax
+    "test_file_existence",
+    "test_error_messages",
+    "test_errors",
+]

test/quick_test/common.py

@@ -19,7 +19,7 @@ def load_test_subjects():
     test_subjects = []
 
     # Load the reference and test files
-    with open(os.path.join(subjects_dir, subjects_list), 'r') as file:
+    with open(os.path.join(subjects_dir, subjects_list), "r") as file:
         for line in file:
             filename = line.strip()
             logger.debug(filename)

test/quick_test/conftest.py

@@ -3,6 +3,7 @@
 
 import pytest
 
+
 @pytest.fixture
 def subjects_dir():
     return Path(os.environ["SUBJECTS_DIR"])
@@ -20,4 +21,4 @@ def reference_dir():
 
 @pytest.fixture
 def subjects_list():
-    return os.environ["SUBJECTS_LIST"]
+    return Path(os.environ["SUBJECTS_LIST"])

test/quick_test/test_errors_in_logfiles.py

@@ -1,4 +1,3 @@
-import os
 import pytest
 import yaml
 from pathlib import Path
@@ -24,23 +23,23 @@ def load_errors():
 
     # Open the error_file_path and read the errors and whitelist into arrays
 
-    error_file_path = os.path.join(Path(__file__).parent, "data/logfile.errors.yaml")
+    error_file_path = Path(__file__).parent / "data" / "logfile.errors.yaml"
 
-    with open(error_file_path, 'r') as file:
+    with open(error_file_path, "r") as file:
         data = yaml.safe_load(file)
-        errors = data.get('errors', [])
-        whitelist = data.get('whitelist', [])
+        errors = data.get("errors", [])
+        whitelist = data.get("whitelist", [])
 
     return errors, whitelist
 
 
-def load_log_files(test_subject: str):
+def load_log_files(test_subject: Path):
     """
     Retrieve the log files in the given log directory.
 
     Parameters
     ----------
-    test_subject : str
+    test_subject : Path
         Subject directory to test.
 
     Returns
@@ -51,24 +50,24 @@ def load_log_files(test_subject: str):
 
     # Retrieve the log files in given log directory
 
-    log_directory = os.path.join(test_subject, "scripts")
-    log_files = [file for file in Path(log_directory).iterdir() if file.suffix == '.log']
+    log_directory = test_subject / "scripts"
+    log_files = [file for file in Path(log_directory).iterdir() if file.suffix == ".log"]
 
     return log_files
 
 
 @pytest.mark.parametrize("test_subject", load_test_subjects())
-def test_errors(subjects_dir, test_dir, test_subject):
+def test_errors(subjects_dir: Path, test_dir: Path, test_subject: Path):
     """
     Test if there are any errors in the log files.
 
     Parameters
     ----------
-    subjects_dir : str
+    subjects_dir : Path
         Subjects directory.
-    test_dir : str
+    test_dir : Path
         Tests directory.
-    test_subject : str
+    test_subject : Path
         Subject to test.
 
     Raises
@@ -77,7 +76,7 @@ def test_errors(subjects_dir, test_dir, test_subject):
         If any of the keywords are in the log files.
     """
 
-    test_subject = os.path.join(subjects_dir, test_dir, test_subject)
+    test_subject = subjects_dir / test_dir / test_subject
     log_files = load_log_files(test_subject)
 
     error_flag = False
@@ -91,14 +90,14 @@ def test_errors(subjects_dir, test_dir, test_subject):
         rel_path = log_file.relative_to(subjects_dir)
         logger.debug(f"Checking file: {rel_path}")
         try:
-            with log_file.open('r') as file:
+            with log_file.open("r") as file:
                 lines = file.readlines()
                 lines_with_errors = []
                 for line_number, line in enumerate(lines, start=1):
                     if any(error in line.lower() for error in errors):
                         if not any(white in line.lower() for white in whitelist):
                             # Get two lines before and after the current line
-                            context = lines[max(0, line_number - 2):min(len(lines), line_number + 3)]
+                            context = lines[max(0, line_number - 2) : min(len(lines), line_number + 3)]
                             lines_with_errors.append((line_number, context))
                             # print(lines_with_errors)
                             files_with_errors[rel_path] = lines_with_errors

test/quick_test/test_file_existence.py

@@ -1,44 +1,21 @@
-import os
 import pytest
-import yaml
 
 from .common import *
 
 from logging import getLogger
 
+from pathlib import Path
+
 logger = getLogger(__name__)
 
 
-# def get_files_from_yaml(file_path: str):
-#     """
-#     Get the list of files from the YAML file.
-#
-#     Parameters
-#     ----------
-#     file_path : str
-#         Path to the YAML file.
-#
-#     Returns
-#     -------
-#     list
-#         List of files specified in the YAML file.
-#     """
-#
-#     # Open the file_path and read the files into an array
-#     with open(file_path, 'r') as file:
-#         data = yaml.safe_load(file)
-#         files = data.get('files', [])
-#
-#     return files
-
-
-def get_files_from_folder(folder_path: str):
+def get_files_from_folder(folder_path: Path):
     """
     Get the list of files in the directory relative to the folder path.
 
     Parameters
     ----------
-    folder_path : str
+    folder_path : Path
         Path to the folder.
 
     Returns
@@ -49,27 +26,26 @@ def get_files_from_folder(folder_path: str):
 
     # Get a list of all files in the folder recursively
     filenames = []
-    for root, _, files in os.walk(folder_path):
-        for file in files:
-            filenames.append(os.path.relpath(os.path.join(root, file), folder_path))
+    for file in Path(folder_path).rglob("*"):
+        filenames.append(str(file.relative_to(folder_path)))
 
     return filenames
 
 
 @pytest.mark.parametrize("test_subject", load_test_subjects())
-def test_file_existence(subjects_dir, test_dir, reference_dir, test_subject):
+def test_file_existence(subjects_dir: Path, test_dir: Path, reference_dir: Path, test_subject: Path):
     """
     Test the existence of files in the folder.
 
     Parameters
     ----------
-    subjects_dir : str
+    subjects_dir : Path
         Path to the subjects directory.
-    test_dir : str
+    test_dir : Path
         Name of the test directory.
-    reference_dir : str
+    reference_dir : Path
         Name of the reference directory.
-    test_subject : str
+    test_subject : Path
         Name of the test subject.
 
     Raises
@@ -81,11 +57,11 @@ def test_file_existence(subjects_dir, test_dir, reference_dir, test_subject):
     print(test_subject)
 
     # Get reference files from the reference subject directory
-    reference_subject = os.path.join(subjects_dir, reference_dir, test_subject)
+    reference_subject = subjects_dir / reference_dir / test_subject
     reference_files = get_files_from_folder(reference_subject)
 
     # Get test list of files in the test subject directory
-    test_subject = os.path.join(subjects_dir, test_dir, test_subject)
+    test_subject = subjects_dir / test_dir / test_subject
     test_files = get_files_from_folder(test_subject)
 
     # Check if each file in the reference list exists in the test list