[FIX] Fix get_spm_version (aramis-lab#1279)

* rework some nipype spm interface config and fix get_spm_version

* fix unit tests

* handle multi-platform in unit test

* try adding spm path to scripts

* try fixing existing folder issues

* some fixes to the statistics-volume pipeline

* try reverting

* another fix

* do not use /mnt/data/ci/tmp as a tmp_path

.github/workflows/test_non_regression_fast.yml

@@ -39,15 +39,10 @@ jobs:
           poetry run pytest --verbose \
           --working_directory=/Volumes/data/working_dir_mac \
           --input_data_directory=/Volumes/data_ci \
-          --basetemp=/Volumes/data/tmp \
           --junitxml=./test-reports/non_regression_fast_mac.xml \
           --disable-warnings \
           -m "fast" \
           ./nonregression/
-      - name: Clean
-        run: |
-          rm -rf /Volumes/data/tmp
-          rm -rf /Volumes/data/working_dir_mac
 
   test-non-regression-fast-Linux:
     runs-on:
@@ -76,12 +71,7 @@ jobs:
           poetry run pytest --verbose \
           --working_directory=/mnt/data/ci/working_dir_linux \
           --input_data_directory=/mnt/data_ci \
-          --basetemp=/mnt/data/ci/tmp \
           --junitxml=./test-reports/non_regression_fast_linux.xml \
           --disable-warnings \
           -m "fast" \
           ./nonregression/
-      - name: Clean
-        run: |
-          rm -rf /mnt/data/ci/tmp
-          rm -rf /mnt/data/ci/working_dir_linux

clinica/pipelines/statistics_volume/statistics_volume_pipeline.py

@@ -1,6 +1,7 @@
 from typing import List
 
 from clinica.pipelines.engine import Pipeline
+from clinica.utils.pet import SUVRReferenceRegion, Tracer
 
 
 class StatisticsVolume(Pipeline):
@@ -31,7 +32,13 @@ def _check_pipeline_parameters(self) -> None:
 
         # Optional parameters for inputs from pet-volume pipeline
         self.parameters.setdefault("acq_label", None)
+        if self.parameters["acq_label"]:
+            self.parameters["acq_label"] = Tracer(self.parameters["acq_label"])
         self.parameters.setdefault("suvr_reference_region", None)
+        if self.parameters["suvr_reference_region"]:
+            self.parameters["suvr_reference_region"] = SUVRReferenceRegion(
+                self.parameters["suvr_reference_region"]
+            )
         self.parameters.setdefault("use_pvc_data", False)
 
         # Optional parameters for custom pipeline
@@ -105,12 +112,12 @@ def _build_input_node(self):
             ):
                 raise ValueError(
                     f"Missing value(s) in parameters from pet-volume pipeline. Given values:\n"
-                    f"- acq_label: {self.parameters['acq_label']}\n"
-                    f"- suvr_reference_region: {self.parameters['suvr_reference_region']}\n"
+                    f"- acq_label: {self.parameters['acq_label'].value}\n"
+                    f"- suvr_reference_region: {self.parameters['suvr_reference_region'].value}\n"
                     f"- use_pvc_data: {self.parameters['use_pvc_data']}\n"
                 )
 
-            self.parameters["measure_label"] = self.parameters["acq_label"]
+            self.parameters["measure_label"] = self.parameters["acq_label"].value
             information_dict = pet_volume_normalized_suvr_pet(
                 acq_label=self.parameters["acq_label"],
                 group_label=self.parameters["group_label_dartel"],

clinica/pipelines/statistics_volume/statistics_volume_utils.py

@@ -522,6 +522,7 @@ def _run_matlab_script_with_matlab(m_file: str) -> None:
 
     from nipype.interfaces.matlab import MatlabCommand, get_matlab_command
 
+    _add_spm_path_to_matlab_script(m_file)
     MatlabCommand.set_default_matlab_cmd(get_matlab_command())
     matlab = MatlabCommand()
     if platform.system().lower().startswith("linux"):
@@ -533,6 +534,16 @@ def _run_matlab_script_with_matlab(m_file: str) -> None:
     matlab.run()
 
 
+def _add_spm_path_to_matlab_script(m_file: str) -> None:
+    from clinica.utils.check_dependency import get_spm_home
+
+    with open(m_file, "r") as fp:
+        lines = fp.readlines()
+    lines.insert(0, f"addpath('{get_spm_home()}');")
+    with open(m_file, "w") as fp:
+        fp.writelines(lines)
+
+
 def clean_template_file(mat_file: str, template_file: str) -> str:
     """Make a copy of the template file (for estimation) and replace
     @SPMMAT by the real path to SPM.mat (mat_file).

clinica/utils/check_dependency.py

@@ -465,18 +465,20 @@ def _get_freesurfer_version() -> Version:
 def _get_spm_version() -> Version:
     from nipype.interfaces import spm
 
+    from .spm import configure_nipype_interface_to_work_with_spm
+
+    configure_nipype_interface_to_work_with_spm()
+
     return Version(spm.SPMCommand().version)
 
 
 def _get_spm_standalone_version() -> Version:
-    import os
-    from pathlib import Path
-
     from nipype.interfaces import spm
 
-    spm_path = Path(os.environ["SPM_HOME"])
-    matlab_cmd = f"{spm_path / 'run_spm12.sh'} {os.environ['MCR_HOME']} script"
-    spm.SPMCommand.set_mlab_paths(matlab_cmd=matlab_cmd, use_mcr=True)
+    from .spm import configure_nipype_interface_to_work_with_spm_standalone
+
+    configure_nipype_interface_to_work_with_spm_standalone()
+
     return Version(spm.SPMCommand().version)
 
 

clinica/utils/spm.py

@@ -9,6 +9,8 @@
     "get_spm_tissue_from_index",
     "get_tpm",
     "use_spm_standalone_if_available",
+    "configure_nipype_interface_to_work_with_spm",
+    "configure_nipype_interface_to_work_with_spm_standalone",
 ]
 
 
@@ -80,39 +82,39 @@ def use_spm_standalone_if_available() -> bool:
     ClinicaEnvironmentVariableError :
         If the environment variables are set to non-existent folders.
     """
-    from clinica.utils.stream import cprint
-
-    from .check_dependency import get_mcr_home, get_spm_home, get_spm_standalone_home
     from .exceptions import ClinicaMissingDependencyError
+    from .stream import log_and_warn
 
     try:
-        spm_standalone_home = get_spm_standalone_home()
-        mcr_home = get_mcr_home()
-        cprint(
-            f"SPM standalone has been found at {spm_standalone_home}, "
-            f"with an MCR at {mcr_home} and will be used in this pipeline"
-        )
-        matlab_command = _get_platform_dependant_matlab_command(
-            spm_standalone_home, mcr_home
-        )
-        _configure_spm_nipype_interface(matlab_command)
+        configure_nipype_interface_to_work_with_spm_standalone()
         return True
     except ClinicaMissingDependencyError:
-        warnings.warn(
-            "SPM standalone is not available on this system. "
-            "The pipeline will try to use SPM and Matlab instead. "
-            "If you want to rely on spm standalone, please make sure "
-            "to set the following environment variables: "
-            "$SPMSTANDALONE_HOME, and $MCR_HOME"
+        log_and_warn(
+            (
+                "SPM standalone is not available on this system. "
+                "The pipeline will try to use SPM and Matlab instead. "
+                "If you want to rely on spm standalone, please make sure "
+                "to set the following environment variables: "
+                "$SPMSTANDALONE_HOME, and $MCR_HOME"
+            ),
+            UserWarning,
         )
-        import nipype.interfaces.matlab as mlab
-
-        cprint(f"Setting SPM path to {get_spm_home()}", lvl="info")
-        mlab.MatlabCommand.set_default_paths(f"{get_spm_home()}")
+        configure_nipype_interface_to_work_with_spm()
         return False
 
 
-def _get_platform_dependant_matlab_command(
+def configure_nipype_interface_to_work_with_spm() -> None:
+    import nipype.interfaces.matlab as mlab
+
+    from clinica.utils.stream import cprint
+
+    from .check_dependency import get_spm_home
+
+    cprint(f"Setting SPM path to {get_spm_home()}", lvl="info")
+    mlab.MatlabCommand.set_default_paths(f"{get_spm_home()}")
+
+
+def _get_platform_dependant_matlab_command_for_spm_standalone(
     spm_standalone_home: Path, mcr_home: Path
 ) -> str:
     import platform
@@ -127,10 +129,23 @@ def _get_platform_dependant_matlab_command(
     )
 
 
-def _configure_spm_nipype_interface(matlab_command: str):
+def configure_nipype_interface_to_work_with_spm_standalone() -> None:
     from nipype.interfaces import spm
 
     from clinica.utils.stream import cprint
 
-    spm.SPMCommand.set_mlab_paths(matlab_cmd=matlab_command, use_mcr=True)
-    cprint(f"Using SPM standalone version {spm.SPMCommand().version}")
+    from .check_dependency import get_mcr_home, get_spm_standalone_home
+
+    spm_standalone_home = get_spm_standalone_home()
+    mcr_home = get_mcr_home()
+    cprint(
+        f"SPM standalone has been found at {spm_standalone_home}, "
+        f"with an MCR at {mcr_home} and will be used in this pipeline"
+    )
+    spm.SPMCommand.set_mlab_paths(
+        matlab_cmd=_get_platform_dependant_matlab_command_for_spm_standalone(
+            spm_standalone_home, mcr_home
+        ),
+        use_mcr=True,
+    )
+    cprint(f"Using SPM standalone version {spm.SPMCommand().version}", lvl="info")

test/nonregression/pipelines/test_run_pipelines_stats.py

@@ -180,8 +180,8 @@ def run_statistics_volume_pet(
         / "groups"
         / "group-UnitTest"
         / "statistics_volume"
-        / "group_comparison_measure-fdg"
-        / "group-UnitTest_CN-lt-AD_measure-fdg_fwhm-8_TStatistics.nii"
+        / f"group_comparison_measure-{Tracer.FDG.value}"
+        / f"group-UnitTest_CN-lt-AD_measure-{Tracer.FDG.value}_fwhm-8_TStatistics.nii"
     )
     ref_t_stat = (
         ref_dir

test/nonregression/testing_tools.py

@@ -18,7 +18,7 @@ def configure_paths(
     input_dir = base_dir / name / "in"
     ref_dir = base_dir / name / "ref"
     tmp_out_dir = tmp_path / name / "out"
-    tmp_out_dir.mkdir(parents=True)
+    tmp_out_dir.mkdir(parents=True, exist_ok=False)
 
     return input_dir, tmp_out_dir, ref_dir
 

test/unittests/utils/test_check_dependency.py

@@ -257,20 +257,30 @@ def test_get_freesurfer_version(mocker):
     assert get_software_version("freesurfer") == Version("1.2.3")
 
 
-def test_get_spm_version():
+def test_get_spm_version(tmp_path):
     from clinica.utils.check_dependency import get_software_version
 
     class SPMVersionMock:
         version: str = "12.6789"
 
-    with mock.patch(
-        "nipype.interfaces.spm.SPMCommand", wraps=SPMVersionMock
-    ) as spm_mock:
-        assert get_software_version("spm") == Version("12.6789")
-        spm_mock.assert_called_once()
+    spm_home_mock = tmp_path / "spm_home"
+    spm_home_mock.mkdir()
+
+    with mock.patch.dict(
+        os.environ,
+        {
+            "SPM_HOME": str(spm_home_mock),
+        },
+    ):
+        with mock.patch(
+            "nipype.interfaces.spm.SPMCommand", wraps=SPMVersionMock
+        ) as spm_mock:
+            assert get_software_version("spm") == Version("12.6789")
+            spm_mock.assert_called_once()
 
 
-def test_get_spm_standalone_version(tmp_path):
+@pytest.mark.parametrize("platform", ["linux", "darwin"])
+def test_get_spm_standalone_version(tmp_path, mocker, platform):
     from clinica.utils.check_dependency import get_software_version
 
     class SPMStandaloneVersionMock:
@@ -279,11 +289,17 @@ class SPMStandaloneVersionMock:
         def set_mlab_paths(self, matlab_cmd: str, use_mcr: bool):
             pass
 
+    spm_standalone_home_mock = tmp_path / "spm_standalone_home"
+    spm_standalone_home_mock.mkdir()
+    mcr_home_mock = tmp_path / "mcr_home"
+    mcr_home_mock.mkdir()
+
+    mocker.patch("platform.system", return_value=platform)
     with mock.patch.dict(
         os.environ,
         {
-            "SPM_HOME": str(tmp_path / "spm_home_mock"),
-            "MCR_HOME": str(tmp_path / "mcr_home_mock"),
+            "SPMSTANDALONE_HOME": str(spm_standalone_home_mock),
+            "MCR_HOME": str(mcr_home_mock),
         },
     ):
         with mock.patch(
@@ -295,7 +311,11 @@ def set_mlab_paths(self, matlab_cmd: str, use_mcr: bool):
                 assert get_software_version("spm standalone") == Version("13.234")
                 spm_mock.set_mlab_paths.assert_called()
                 mock_method.assert_called_once_with(
-                    matlab_cmd=f"{tmp_path / 'spm_home_mock' / 'run_spm12.sh'} {tmp_path / 'mcr_home_mock'} script",
+                    matlab_cmd=(
+                        f"cd {tmp_path / 'spm_standalone_home'} && ./run_spm12.sh {tmp_path / 'mcr_home'} script"
+                        if platform == "darwin"
+                        else f"{tmp_path / 'spm_standalone_home' / 'run_spm12.sh'} {tmp_path / 'mcr_home'} script"
+                    ),
                     use_mcr=True,
                 )
 

test/unittests/utils/test_spm.py

@@ -69,7 +69,10 @@ def test_spm_standalone_is_available_warning(tmp_path):
 def test_spm_standalone_is_available(tmp_path, mocker):
     from clinica.utils.spm import use_spm_standalone_if_available
 
-    mocker.patch("clinica.utils.spm._configure_spm_nipype_interface", return_value=None)
+    mocker.patch(
+        "clinica.utils.spm.configure_nipype_interface_to_work_with_spm_standalone",
+        return_value=None,
+    )
     with mock.patch.dict(
         os.environ,
         {
@@ -110,24 +113,34 @@ def test_use_spm_standalone_if_available_error(tmp_path):
         ("Linux", "/foo/bar/run_spm12.sh /foo/bar/baz script"),
     ],
 )
-def test_get_platform_dependant_matlab_command(mocker, platform, expected_command):
-    from clinica.utils.spm import _get_platform_dependant_matlab_command  # noqa
+def test_get_platform_dependant_matlab_command_for_spm_standalone(
+    mocker, platform, expected_command
+):
+    from clinica.utils.spm import (
+        _get_platform_dependant_matlab_command_for_spm_standalone,
+    )
 
     mocker.patch("platform.system", return_value=platform)
 
     assert (
-        _get_platform_dependant_matlab_command(Path("/foo/bar"), Path("/foo/bar/baz"))
+        _get_platform_dependant_matlab_command_for_spm_standalone(
+            Path("/foo/bar"), Path("/foo/bar/baz")
+        )
         == expected_command
     )
 
 
-def test_get_platform_dependant_matlab_command_error(mocker):
-    from clinica.utils.spm import _get_platform_dependant_matlab_command  # noqa
+def test_get_platform_dependant_matlab_command_for_spm_standalone_error(mocker):
+    from clinica.utils.spm import (
+        _get_platform_dependant_matlab_command_for_spm_standalone,
+    )
 
     mocker.patch("platform.system", return_value="foo")
 
     with pytest.raises(
         SystemError,
         match="Clinica only support macOS and Linux. Your system is foo.",
     ):
-        _get_platform_dependant_matlab_command(Path("/foo/bar"), Path("/foo/bar/baz"))
+        _get_platform_dependant_matlab_command_for_spm_standalone(
+            Path("/foo/bar"), Path("/foo/bar/baz")
+        )