From ea4988b08bca1daa5818aa0ecd3d27d899c8f25d Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:24:43 +0200 Subject: [PATCH 01/30] refactor: move Singleton to its own module under junifer/pipeline --- junifer/markers/utils.py | 41 +------------------------------ junifer/pipeline/singleton.py | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 40 deletions(-) create mode 100644 junifer/pipeline/singleton.py diff --git a/junifer/markers/utils.py b/junifer/markers/utils.py index 2b5f3ded0..7e785a174 100644 --- a/junifer/markers/utils.py +++ b/junifer/markers/utils.py @@ -7,7 +7,7 @@ # Federico Raimondo # License: AGPL -from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union +from typing import Callable, List, Optional, Tuple, Union import numpy as np import pandas as pd @@ -16,45 +16,6 @@ from ..utils import raise_error -def singleton(cls: Type) -> Type: - """Make a class singleton. - - Parameters - ---------- - cls : class - The class to designate as singleton. - - Returns - ------- - class - The only instance of the class. - - """ - instances: Dict = {} - - def get_instance(*args: Any, **kwargs: Any) -> Type: - """Get the only instance for a class. - - Parameters - ---------- - *args : tuple - The positional arguments to pass to the class. - **kwargs : dict - The keyword arguments to pass to the class. - - Returns - ------- - class - The only instance of the class. - - """ - if cls not in instances: - instances[cls] = cls(*args, **kwargs) - return instances[cls] - - return get_instance - - def _ets( bold_ts: np.ndarray, roi_names: Union[None, List[str]] = None, diff --git a/junifer/pipeline/singleton.py b/junifer/pipeline/singleton.py new file mode 100644 index 000000000..b04d7507d --- /dev/null +++ b/junifer/pipeline/singleton.py @@ -0,0 +1,45 @@ +"""Provide a singleton class to be used by pipeline components.""" + +# Authors: Synchon Mandal +# License: AGPL + +from typing import Any, Dict, Type + + +def singleton(cls: Type) -> Type: + """Make a class singleton. + + Parameters + ---------- + cls : class + The class to designate as singleton. + + Returns + ------- + class + The only instance of the class. + + """ + instances: Dict = {} + + def get_instance(*args: Any, **kwargs: Any) -> Type: + """Get the only instance for a class. + + Parameters + ---------- + *args : tuple + The positional arguments to pass to the class. + **kwargs : dict + The keyword arguments to pass to the class. + + Returns + ------- + class + The only instance of the class. + + """ + if cls not in instances: + instances[cls] = cls(*args, **kwargs) + return instances[cls] + + return get_instance From b0869b839d441523e804ce7a5bbc53947757b8dd Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:26:40 +0200 Subject: [PATCH 02/30] update: adapt ReHoEstimator and ALFFEstimator for Singleton refactoring --- junifer/markers/falff/falff_estimator.py | 2 +- junifer/markers/reho/reho_estimator.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index f6bf57cae..8f35f9ab1 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -17,8 +17,8 @@ from nilearn import image as nimg from scipy.fft import fft, fftfreq +from ...pipeline.singleton import singleton from ...utils import logger, raise_error -from ..utils import singleton if TYPE_CHECKING: diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 211c5d44c..5e8fd4897 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -20,8 +20,8 @@ from nilearn import masking as nmask from scipy.stats import rankdata +from ...pipeline.singleton import singleton from ...utils import logger, raise_error -from ..utils import singleton if TYPE_CHECKING: From 8ee46ffa72cf89e6714e8d3c7c170c3a39ac03ba Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:28:01 +0200 Subject: [PATCH 03/30] feature: introduce WorkDirManager for a global way to handle tempdirs --- junifer/pipeline/workdir_manager.py | 112 ++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 junifer/pipeline/workdir_manager.py diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py new file mode 100644 index 000000000..b64fca613 --- /dev/null +++ b/junifer/pipeline/workdir_manager.py @@ -0,0 +1,112 @@ +"""Provide a work directory manager class to be used by pipeline components.""" + +# Authors: Synchon Mandal +# Federico Raimondo +# License: AGPL + +import shutil +import tempfile +from pathlib import Path +from typing import Optional, Union + +from ..utils import logger +from .singleton import singleton + + +@singleton +class WorkDirManager: + """Class for working directory manager. + + This class is a singleton and is used for managing temporary and working + directories used across the pipeline by datagrabbers, preprocessors, + markers and so on. It maintains a single super-directory and provides + directories on-demand and cleans after itself thus keeping the user + filesystem clean. + + + Parameters + ---------- + workdir : str or pathlib.Path, optional + The path to the super-directory. If None, "TMPDIR/junifer" is used + where TMPDIR is the platform-dependent temporary directory. + + """ + + def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: + """Initialize the class.""" + # Check if workdir is already set + if not hasattr(self, "_workdir"): + # Convert str to Path + if isinstance(workdir, str): + workdir = Path(workdir) + # Check and set topmost level directory if not provided + if workdir is None: + workdir = Path(tempfile.gettempdir()) + logger.debug(f"Setting working directory at {workdir.resolve()!s}") + self._workdir = workdir + + # Initialize root temporary directory + self._root_temp_dir: Optional[Path] = None + + def __del__(self) -> None: + """Destructor.""" + # Remove root temporary directory + if self._root_temp_dir is not None: + shutil.rmtree(self._root_temp_dir, ignore_errors=True) + + @property + def workdir(self) -> Path: + """Get working directory.""" + return self._workdir + + @workdir.setter + def workdir(self, value: Path) -> None: + """Set working directory.""" + logger.debug(f"Changing working directory to {value}") + self._workdir = value + + def get_tempdir( + self, prefix: Optional[str] = None, suffix: Optional[str] = None + ) -> Path: + """Get a temporary directory. + + Parameters + ---------- + prefix : str + The temporary directory prefix. If None, a default prefix is used. + suffix : str + The temporary directory suffix. If None, no suffix is added. + + Returns + ------- + pathlib.Path + The path to the temporary directory. + + """ + # Create root temporary directory if not created already + if self._root_temp_dir is None: + logger.debug( + f"Setting up temporary directory under {self._workdir}" + ) + self._root_temp_dir = Path( + tempfile.mkdtemp(prefix=prefix, dir=self._workdir) + ) + + logger.debug(f"Creating temporary directory at {self._root_temp_dir}") + return Path( + tempfile.mkdtemp( + dir=self._root_temp_dir, prefix=prefix, suffix=suffix + ) + ) + + def delete_tempdir(self, tempdir: Path) -> None: + """Delete a temporary directory. + + Parameters + ---------- + tempdir : pathlib.Path + The temporary directory path to be deleted. + + """ + logger.debug(f"Deleting temporary directory at {tempdir}") + shutil.rmtree(tempdir, ignore_errors=True) From 1c080efd27dc5d7d1786be0ae21eb52bcb4b1491 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:28:55 +0200 Subject: [PATCH 04/30] update: adapt ReHoEstimator to use WorkDirManager --- junifer/markers/reho/reho_estimator.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 5e8fd4897..721b2464b 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -6,12 +6,9 @@ import hashlib -import shutil import subprocess -import tempfile from functools import lru_cache from itertools import product -from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional, cast import nibabel as nib @@ -20,6 +17,7 @@ from nilearn import masking as nmask from scipy.stats import rankdata +from ...pipeline import WorkDirManager from ...pipeline.singleton import singleton from ...utils import logger, raise_error @@ -50,12 +48,12 @@ def __init__(self) -> None: self._file_path = None # Create temporary directory for intermittent storage of assets during # computation via afni's 3dReHo - self.temp_dir_path = Path(tempfile.mkdtemp()) + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="reho") def __del__(self) -> None: """Cleanup.""" # Delete temporary directory and ignore errors for read-only files - shutil.rmtree(self.temp_dir_path, ignore_errors=True) + WorkDirManager().delete_tempdir(self.temp_dir_path) def _compute_reho_afni( self, From d708d8b8aa91d246fa125733f562d80cda565950 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:30:30 +0200 Subject: [PATCH 05/30] update: adapt ALFFEstimator to use WorkDirManager --- junifer/markers/falff/falff_estimator.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 8f35f9ab1..01158cc7c 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -4,12 +4,9 @@ # Federico Raimondo # License: AGPL -import shutil import subprocess -import tempfile import typing from functools import lru_cache -from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple, Union import nibabel as nib @@ -17,6 +14,7 @@ from nilearn import image as nimg from scipy.fft import fft, fftfreq +from ...pipeline import WorkDirManager from ...pipeline.singleton import singleton from ...utils import logger, raise_error @@ -47,13 +45,13 @@ def __init__(self) -> None: self._file_path = None # Create temporary directory for intermittent storage of assets during # computation via afni's 3dReHo - self.temp_dir_path = Path(tempfile.mkdtemp()) + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") def __del__(self) -> None: """Cleanup.""" print("Cleaning up temporary directory...") # Delete temporary directory and ignore errors for read-only files - shutil.rmtree(self.temp_dir_path, ignore_errors=True) + WorkDirManager().delete_tempdir(self.temp_dir_path) @staticmethod def _run_afni_cmd(cmd: str) -> None: From 5e8101e677786c14a99cb82eda1c5ff2794adad2 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:30:40 +0200 Subject: [PATCH 06/30] chore: improve commentary for ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 01158cc7c..5a68b1a7e 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -44,7 +44,7 @@ class ALFFEstimator: def __init__(self) -> None: self._file_path = None # Create temporary directory for intermittent storage of assets during - # computation via afni's 3dReHo + # computation via afni's 3dRSFC self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") def __del__(self) -> None: From 33a9c4973c9759b079f1f9a71afc38b3a3fb31bd Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:30:57 +0200 Subject: [PATCH 07/30] chore: improve docstring for ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 5a68b1a7e..feea6e9ae 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -39,6 +39,11 @@ class ALFFEstimator: use_afni : bool Whether to use afni for computation. If False, will use python. + Attributes + ---------- + temp_dir_path : pathlib.Path + Path to the temporary directory for assets storage. + """ def __init__(self) -> None: From 8ef065a9da2ccddabc934ab2e617b0b4cd5924d7 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:31:16 +0200 Subject: [PATCH 08/30] chore: remove stray print statement from ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index feea6e9ae..9c997ff58 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -54,7 +54,6 @@ def __init__(self) -> None: def __del__(self) -> None: """Cleanup.""" - print("Cleaning up temporary directory...") # Delete temporary directory and ignore errors for read-only files WorkDirManager().delete_tempdir(self.temp_dir_path) From d0f962589192b9a97c1f96cb90aa9e2e358c03df Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:31:37 +0200 Subject: [PATCH 09/30] update: expose WorkDirManager to pipeline sub-package level --- junifer/pipeline/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/junifer/pipeline/__init__.py b/junifer/pipeline/__init__.py index 8d1681388..1ea157d60 100644 --- a/junifer/pipeline/__init__.py +++ b/junifer/pipeline/__init__.py @@ -6,3 +6,4 @@ from . import registry from .pipeline_step_mixin import PipelineStepMixin from .update_meta_mixin import UpdateMetaMixin +from .workdir_manager import WorkDirManager From 5ac7fd49e973f44a014253ad66d454d08bb01b54 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 4 Oct 2023 16:32:03 +0200 Subject: [PATCH 10/30] update: use WorkDirManager in api.functions.run() --- junifer/api/functions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/junifer/api/functions.py b/junifer/api/functions.py index 37e94ad2d..a64ad469b 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -15,6 +15,7 @@ from ..datagrabber.base import BaseDataGrabber from ..markers.base import BaseMarker from ..markers.collection import MarkerCollection +from ..pipeline import WorkDirManager from ..pipeline.registry import build from ..preprocess.base import BasePreprocessor from ..storage.base import BaseFeatureStorage @@ -115,6 +116,8 @@ def run( # Convert str to Path if isinstance(workdir, str): workdir = Path(workdir) + # Initiate working directory manager + WorkDirManager(workdir) if not isinstance(elements, list) and elements is not None: elements = [elements] From 7e3cfa808d9958967dec525e800be17e2e0684c9 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Thu, 5 Oct 2023 13:16:25 +0200 Subject: [PATCH 11/30] chore: slight refactor of WorkDirManager init --- junifer/pipeline/workdir_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index b64fca613..c492b8ada 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -36,12 +36,12 @@ def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: """Initialize the class.""" # Check if workdir is already set if not hasattr(self, "_workdir"): - # Convert str to Path - if isinstance(workdir, str): - workdir = Path(workdir) # Check and set topmost level directory if not provided if workdir is None: workdir = Path(tempfile.gettempdir()) + # Convert str to Path + if isinstance(workdir, str): + workdir = Path(workdir) logger.debug(f"Setting working directory at {workdir.resolve()!s}") self._workdir = workdir From d2065aa8a2eb69f4a9b360369768d7f4a629fafa Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Thu, 5 Oct 2023 13:36:25 +0200 Subject: [PATCH 12/30] fix: remove prefix from root temp dir creation for WorkDirManager --- junifer/pipeline/workdir_manager.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index c492b8ada..b15575b4f 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -88,9 +88,7 @@ def get_tempdir( logger.debug( f"Setting up temporary directory under {self._workdir}" ) - self._root_temp_dir = Path( - tempfile.mkdtemp(prefix=prefix, dir=self._workdir) - ) + self._root_temp_dir = Path(tempfile.mkdtemp(dir=self._workdir)) logger.debug(f"Creating temporary directory at {self._root_temp_dir}") return Path( From 9dc4aa42069a000df2dc656cd00607c0ce9d104f Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Thu, 5 Oct 2023 13:38:02 +0200 Subject: [PATCH 13/30] fix: remove workdir setter for WorkDirManager --- junifer/pipeline/workdir_manager.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index b15575b4f..87ff34975 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -59,12 +59,6 @@ def workdir(self) -> Path: """Get working directory.""" return self._workdir - @workdir.setter - def workdir(self, value: Path) -> None: - """Set working directory.""" - logger.debug(f"Changing working directory to {value}") - self._workdir = value - def get_tempdir( self, prefix: Optional[str] = None, suffix: Optional[str] = None ) -> Path: From f5caf301fb142ecd95d85f57490b57c3b05f2529 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Thu, 5 Oct 2023 18:34:42 +0200 Subject: [PATCH 14/30] update: add cleanup and refactor logic for WorkDirManager --- junifer/pipeline/workdir_manager.py | 54 +++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index 87ff34975..1921813e0 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -9,7 +9,7 @@ from pathlib import Path from typing import Optional, Union -from ..utils import logger +from ..utils import logger, raise_error from .singleton import singleton @@ -29,12 +29,24 @@ class WorkDirManager: workdir : str or pathlib.Path, optional The path to the super-directory. If None, "TMPDIR/junifer" is used where TMPDIR is the platform-dependent temporary directory. + cleanup : bool, optional + Whether to clean up the old instance when a new instance is requested + (default False). + + Raises + ------ + RuntimeError + If ``cleanup=False`` and a new instance is requested with a different + ``workdir`` than the old instance. """ - def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: + def __init__( + self, workdir: Optional[Union[str, Path]] = None, cleanup: bool = False + ) -> None: """Initialize the class.""" # Check if workdir is already set + # This runs for the first time if not hasattr(self, "_workdir"): # Check and set topmost level directory if not provided if workdir is None: @@ -44,14 +56,52 @@ def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: workdir = Path(workdir) logger.debug(f"Setting working directory at {workdir.resolve()!s}") self._workdir = workdir + # This runs for consecutive times + else: + # Use same workdir if not provided + if workdir is None: + logger.debug( + "Using existing working directory at " + f"{self._workdir.resolve()!s}" + ) + # Check if workdir is changing + else: + # Convert str to Path + if isinstance(workdir, str): + workdir = Path(workdir) + # Check if the existing and the new paths are same + if self._workdir != workdir: + if not cleanup: + raise_error( + msg=( + "Set `cleanup=True` and try again to properly " + "clean up and initialize new working " + "directory." + ), + klass=RuntimeError, + ) + # Clean up and initialize new working directory + self._cleanup() + + logger.debug( + f"Setting working directory at {workdir.resolve()!s}" + ) + self._workdir = workdir # Initialize root temporary directory self._root_temp_dir: Optional[Path] = None def __del__(self) -> None: """Destructor.""" + self._cleanup() + + def _cleanup(self) -> None: + """Clean up the temporary directories.""" # Remove root temporary directory if self._root_temp_dir is not None: + logger.debug( + f"Deleting temporary directory at {self._root_temp_dir}" + ) shutil.rmtree(self._root_temp_dir, ignore_errors=True) @property From f83d42852d89476918e36256b0a03f08b9241248 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 14:48:22 +0200 Subject: [PATCH 15/30] Revert "fix: remove workdir setter for WorkDirManager" This reverts commit 9dc4aa42069a000df2dc656cd00607c0ce9d104f. --- junifer/pipeline/workdir_manager.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index 1921813e0..83e7b8461 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -109,6 +109,12 @@ def workdir(self) -> Path: """Get working directory.""" return self._workdir + @workdir.setter + def workdir(self, value: Path) -> None: + """Set working directory.""" + logger.debug(f"Changing working directory to {value}") + self._workdir = value + def get_tempdir( self, prefix: Optional[str] = None, suffix: Optional[str] = None ) -> Path: From 1ed365b140760fd262ae9a66e3334e374f299b49 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 16:18:05 +0200 Subject: [PATCH 16/30] update: remove cleanup param from WorkDirManager --- junifer/pipeline/workdir_manager.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index 83e7b8461..327ada0b4 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -9,7 +9,7 @@ from pathlib import Path from typing import Optional, Union -from ..utils import logger, raise_error +from ..utils import logger from .singleton import singleton @@ -23,27 +23,16 @@ class WorkDirManager: directories on-demand and cleans after itself thus keeping the user filesystem clean. - Parameters ---------- workdir : str or pathlib.Path, optional The path to the super-directory. If None, "TMPDIR/junifer" is used where TMPDIR is the platform-dependent temporary directory. - cleanup : bool, optional - Whether to clean up the old instance when a new instance is requested - (default False). - Raises - ------ - RuntimeError - If ``cleanup=False`` and a new instance is requested with a different - ``workdir`` than the old instance. """ - def __init__( - self, workdir: Optional[Union[str, Path]] = None, cleanup: bool = False - ) -> None: + def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: """Initialize the class.""" # Check if workdir is already set # This runs for the first time From ddbdf44138c601f9ed944803cdd9734090437903 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 16:19:56 +0200 Subject: [PATCH 17/30] refactor: rename _root_temp_dir to _root_tempdir to make it consistent in WorkDirManager --- junifer/pipeline/workdir_manager.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index 327ada0b4..9b1b4d4f9 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -76,9 +76,8 @@ def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: f"Setting working directory at {workdir.resolve()!s}" ) self._workdir = workdir + self._root_tempdir = None - # Initialize root temporary directory - self._root_temp_dir: Optional[Path] = None def __del__(self) -> None: """Destructor.""" @@ -87,11 +86,12 @@ def __del__(self) -> None: def _cleanup(self) -> None: """Clean up the temporary directories.""" # Remove root temporary directory - if self._root_temp_dir is not None: + if self._root_tempdir is not None: logger.debug( - f"Deleting temporary directory at {self._root_temp_dir}" + "Deleting temporary directory at " + f"{self._root_tempdir.resolve()!s}" ) - shutil.rmtree(self._root_temp_dir, ignore_errors=True) + shutil.rmtree(self._root_tempdir, ignore_errors=True) @property def workdir(self) -> Path: @@ -123,16 +123,18 @@ def get_tempdir( """ # Create root temporary directory if not created already - if self._root_temp_dir is None: + if self._root_tempdir is None: logger.debug( f"Setting up temporary directory under {self._workdir}" ) - self._root_temp_dir = Path(tempfile.mkdtemp(dir=self._workdir)) + self._root_tempdir = Path(tempfile.mkdtemp(dir=self._workdir)) - logger.debug(f"Creating temporary directory at {self._root_temp_dir}") + logger.debug( + f"Creating temporary directory at {self._root_tempdir.resolve()!s}" + ) return Path( tempfile.mkdtemp( - dir=self._root_temp_dir, prefix=prefix, suffix=suffix + dir=self._root_tempdir, prefix=prefix, suffix=suffix ) ) From 65562289e00c686f5c030f9c15a3ec4627bb9d6c Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 16:21:04 +0200 Subject: [PATCH 18/30] update: simplify and improve WorkDirManager logic --- junifer/pipeline/workdir_manager.py | 105 +++++++++++++++------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/junifer/pipeline/workdir_manager.py b/junifer/pipeline/workdir_manager.py index 9b1b4d4f9..4f2b0c787 100644 --- a/junifer/pipeline/workdir_manager.py +++ b/junifer/pipeline/workdir_manager.py @@ -29,55 +29,37 @@ class WorkDirManager: The path to the super-directory. If None, "TMPDIR/junifer" is used where TMPDIR is the platform-dependent temporary directory. + Attributes + ---------- + workdir : pathlib.Path + The path to the working directory. + root_tempdir : pathlib.Path or None + The path to the root temporary directory. """ def __init__(self, workdir: Optional[Union[str, Path]] = None) -> None: """Initialize the class.""" - # Check if workdir is already set - # This runs for the first time - if not hasattr(self, "_workdir"): - # Check and set topmost level directory if not provided - if workdir is None: - workdir = Path(tempfile.gettempdir()) - # Convert str to Path - if isinstance(workdir, str): - workdir = Path(workdir) - logger.debug(f"Setting working directory at {workdir.resolve()!s}") - self._workdir = workdir - # This runs for consecutive times - else: - # Use same workdir if not provided - if workdir is None: - logger.debug( - "Using existing working directory at " - f"{self._workdir.resolve()!s}" - ) - # Check if workdir is changing - else: - # Convert str to Path - if isinstance(workdir, str): - workdir = Path(workdir) - # Check if the existing and the new paths are same - if self._workdir != workdir: - if not cleanup: - raise_error( - msg=( - "Set `cleanup=True` and try again to properly " - "clean up and initialize new working " - "directory." - ), - klass=RuntimeError, - ) - # Clean up and initialize new working directory - self._cleanup() + self._workdir = workdir + self._root_tempdir = None + + self._set_default_workdir() + def _set_default_workdir(self) -> None: + """Set the default working directory if not set already.""" + # Check and set topmost level directory if not provided + if self._workdir is None: + self._workdir = Path(tempfile.gettempdir()) / "junifer" + # Create directory if not found + if not self._workdir.is_dir(): logger.debug( - f"Setting working directory at {workdir.resolve()!s}" + "Creating working directory at " + f"{self._workdir.resolve()!s}" ) - self._workdir = workdir - self._root_tempdir = None - + self._workdir.mkdir(parents=True) + logger.debug( + f"Setting working directory to {self._workdir.resolve()!s}" + ) def __del__(self) -> None: """Destructor.""" @@ -92,17 +74,45 @@ def _cleanup(self) -> None: f"{self._root_tempdir.resolve()!s}" ) shutil.rmtree(self._root_tempdir, ignore_errors=True) + self._root_tempdir = None @property def workdir(self) -> Path: """Get working directory.""" - return self._workdir + return self._workdir # type: ignore @workdir.setter - def workdir(self, value: Path) -> None: - """Set working directory.""" - logger.debug(f"Changing working directory to {value}") - self._workdir = value + def workdir(self, path: Union[str, Path]) -> None: + """Set working directory. + + The directory path is created if it doesn't exist yet. + + Parameters + ---------- + path : str or pathlib.Path + The path to the working directory. + + """ + # Check if existing working directory is same or not; + # if not, then clean up + if self._workdir != Path(path): + self._cleanup() + # Set working directory + self._workdir = Path(path) + logger.debug( + f"Changing working directory to {self._workdir.resolve()!s}" + ) + # Create directory if not found + if not self._workdir.is_dir(): + logger.debug( + f"Creating working directory at {self._workdir.resolve()!s}" + ) + self._workdir.mkdir(parents=True) + + @property + def root_tempdir(self) -> Optional[Path]: + """Get root temporary directory.""" + return self._root_tempdir def get_tempdir( self, prefix: Optional[str] = None, suffix: Optional[str] = None @@ -125,7 +135,8 @@ def get_tempdir( # Create root temporary directory if not created already if self._root_tempdir is None: logger.debug( - f"Setting up temporary directory under {self._workdir}" + "Setting up temporary directory under " + f"{self._workdir.resolve()!s}" # type: ignore ) self._root_tempdir = Path(tempfile.mkdtemp(dir=self._workdir)) From 962fa067d224c5f2c8c16b7ce298267fbe4f39c4 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 16:32:19 +0200 Subject: [PATCH 19/30] update: add WorkDirManager._cleanup() for ReHoEstimator and ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 1 + junifer/markers/reho/reho_estimator.py | 1 + 2 files changed, 2 insertions(+) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 9c997ff58..5f52ef15a 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -56,6 +56,7 @@ def __del__(self) -> None: """Cleanup.""" # Delete temporary directory and ignore errors for read-only files WorkDirManager().delete_tempdir(self.temp_dir_path) + WorkDirManager()._cleanup() @staticmethod def _run_afni_cmd(cmd: str) -> None: diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 721b2464b..75f67ab03 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -54,6 +54,7 @@ def __del__(self) -> None: """Cleanup.""" # Delete temporary directory and ignore errors for read-only files WorkDirManager().delete_tempdir(self.temp_dir_path) + WorkDirManager()._cleanup() def _compute_reho_afni( self, From 10dd7c5f204a278f03a4c605b49f7aace261cf9b Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 16:33:18 +0200 Subject: [PATCH 20/30] update: add tests for WorkDirManager --- .../pipeline/tests/test_workdir_manager.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 junifer/pipeline/tests/test_workdir_manager.py diff --git a/junifer/pipeline/tests/test_workdir_manager.py b/junifer/pipeline/tests/test_workdir_manager.py new file mode 100644 index 000000000..f969c9e20 --- /dev/null +++ b/junifer/pipeline/tests/test_workdir_manager.py @@ -0,0 +1,60 @@ +"""Provide tests for WorkDirManager.""" + +# Authors: Synchon Mandal +# License: AGPL + +import tempfile +from pathlib import Path + +from junifer.pipeline import WorkDirManager + + +def test_workdir_manager_singleton() -> None: + """Test that WorkDirManager is a singleton.""" + workdir_mgr_1 = WorkDirManager() + workdir_mgr_2 = WorkDirManager() + assert id(workdir_mgr_1) == id(workdir_mgr_2) + + +def test_workdir_manager_auto_workdir() -> None: + """Test that automatic workdir creation is in temporary directory.""" + workdir_mgr = WorkDirManager() + assert workdir_mgr.workdir == Path(tempfile.gettempdir()) / "junifer" + + +def test_workdir_manager_explicit_workdir(tmp_path: Path) -> None: + """Test that WorkDirManager works correctly with an explicit workdir. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + workdir_mgr = WorkDirManager() + workdir_mgr.workdir = tmp_path + assert workdir_mgr.workdir == tmp_path + + +def test_workdir_manager_get_and_delete_tempdir(tmp_path: Path) -> None: + """Test getting and deleting temporary directories. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + workdir_mgr = WorkDirManager() + workdir_mgr.workdir = tmp_path + # Check no root temporary directory + assert workdir_mgr.root_tempdir is None + + tempdir = workdir_mgr.get_tempdir() + # Should create a temporary directory + assert workdir_mgr.root_tempdir is not None + + workdir_mgr.delete_tempdir(tempdir) + workdir_mgr._cleanup() + # Should remove temporary directory + assert workdir_mgr.root_tempdir is None From 829d5fb9a0131274c26b4404829f1dafbacc5df4 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 17:36:29 +0200 Subject: [PATCH 21/30] fix: update WorkDirManager in fALFF tests --- .../falff/tests/test_falff_estimator.py | 39 +++++++++++++++--- .../markers/falff/tests/test_falff_parcels.py | 36 ++++++++++++----- .../markers/falff/tests/test_falff_spheres.py | 40 ++++++++++++++----- 3 files changed, 89 insertions(+), 26 deletions(-) diff --git a/junifer/markers/falff/tests/test_falff_estimator.py b/junifer/markers/falff/tests/test_falff_estimator.py index d9e64b5b0..01e39c560 100644 --- a/junifer/markers/falff/tests/test_falff_estimator.py +++ b/junifer/markers/falff/tests/test_falff_estimator.py @@ -4,6 +4,7 @@ # License: AGPL import time +from pathlib import Path import pytest from nibabel import Nifti1Image @@ -11,18 +12,27 @@ from junifer.datareader import DefaultDataReader from junifer.markers.falff.falff_estimator import ALFFEstimator +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.testing.datagrabbers import PartlyCloudyTestingDataGrabber from junifer.utils import logger -def test_ALFFEstimator_cache_python() -> None: - """Test that the cache works properly when using python.""" +def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: + """Test that the cache works properly when using python. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) + WorkDirManager().workdir = tmp_path estimator = ALFFEstimator() start_time = time.time() alff, falff = estimator.fit_transform( @@ -109,13 +119,21 @@ def test_ALFFEstimator_cache_python() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_ALFFEstimator_cache_afni() -> None: - """Test that the cache works properly when using afni.""" +def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: + """Test that the cache works properly when using afni. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) + WorkDirManager().workdir = tmp_path estimator = ALFFEstimator() start_time = time.time() alff, falff = estimator.fit_transform( @@ -202,12 +220,21 @@ def test_ALFFEstimator_cache_afni() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_ALFFEstimator_afni_vs_python() -> None: - """Test that the cache works properly when using afni.""" +def test_ALFFEstimator_afni_vs_python(tmp_path: Path) -> None: + """Test that the cache works properly when using afni. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) + + WorkDirManager().workdir = tmp_path estimator = ALFFEstimator() # Use an arbitrary TR to test the AFNI vs Python implementation diff --git a/junifer/markers/falff/tests/test_falff_parcels.py b/junifer/markers/falff/tests/test_falff_parcels.py index be5fac3be..b6ec7cdf1 100644 --- a/junifer/markers/falff/tests/test_falff_parcels.py +++ b/junifer/markers/falff/tests/test_falff_parcels.py @@ -12,6 +12,7 @@ from junifer.datareader import DefaultDataReader from junifer.markers.falff import ALFFParcels +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.storage import SQLiteFeatureStorage from junifer.testing.datagrabbers import PartlyCloudyTestingDataGrabber @@ -21,15 +22,20 @@ _PARCELLATION = "Schaefer100x7" -def test_ALFFParcels_python() -> None: - """Test ALFFParcels using python.""" - # Get the SPM auditory data: +def test_ALFFParcels_python(tmp_path: Path) -> None: + """Test ALFFParcels using python. + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + WorkDirManager().workdir = tmp_path marker = ALFFParcels( parcellation=_PARCELLATION, method="mean", @@ -46,14 +52,20 @@ def test_ALFFParcels_python() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_ALFFParcels_afni() -> None: - """Test ALFFParcels using afni.""" - # Get the SPM auditory data: +def test_ALFFParcels_afni(tmp_path: Path) -> None: + """Test ALFFParcels using afni. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + WorkDirManager().workdir = tmp_path marker = ALFFParcels( parcellation=_PARCELLATION, method="mean", @@ -83,12 +95,15 @@ def test_ALFFParcels_afni() -> None: "fractional", [True, False], ids=["fractional", "non-fractional"] ) def test_ALFFParcels_python_vs_afni( + tmp_path: Path, fractional: bool, ) -> None: """Test ALFFParcels using python. Parameters ---------- + tmp_path : pathlib.Path + The path to the test directory. fractional : bool Whether to compute fractional ALFF or not. @@ -98,7 +113,7 @@ def test_ALFFParcels_python_vs_afni( input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + WorkDirManager().workdir = tmp_path marker_python = ALFFParcels( parcellation=_PARCELLATION, method="mean", @@ -137,12 +152,13 @@ def test_ALFFParcels_storage( ---------- tmp_path : pathlib.Path The path to the test directory. + """ with PartlyCloudyTestingDataGrabber() as dg: # Use first subject input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + WorkDirManager().workdir = tmp_path marker = ALFFParcels( parcellation=_PARCELLATION, method="mean", diff --git a/junifer/markers/falff/tests/test_falff_spheres.py b/junifer/markers/falff/tests/test_falff_spheres.py index 556cd3844..825dc6a40 100644 --- a/junifer/markers/falff/tests/test_falff_spheres.py +++ b/junifer/markers/falff/tests/test_falff_spheres.py @@ -12,6 +12,7 @@ from junifer.datareader import DefaultDataReader from junifer.markers.falff import ALFFSpheres +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.storage import SQLiteFeatureStorage from junifer.testing.datagrabbers import PartlyCloudyTestingDataGrabber @@ -21,15 +22,21 @@ _COORDINATES = "DMNBuckner" -def test_ALFFSpheres_python() -> None: - """Test ALFFSpheres using python.""" - # Get the SPM auditory data: +def test_ALFFSpheres_python(tmp_path: Path) -> None: + """Test ALFFSpheres using python. + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + + WorkDirManager().workdir = tmp_path marker = ALFFSpheres( coords=_COORDINATES, radius=5, @@ -47,14 +54,21 @@ def test_ALFFSpheres_python() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_ALFFSpheres_afni() -> None: - """Test ALFFSpheres using afni.""" - # Get the SPM auditory data: +def test_ALFFSpheres_afni(tmp_path: Path) -> None: + """Test ALFFSpheres using afni. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + + WorkDirManager().workdir = tmp_path marker = ALFFSpheres( coords=_COORDINATES, radius=5, @@ -88,20 +102,25 @@ def test_ALFFSpheres_afni() -> None: "fractional", [True, False], ids=["fractional", "non-fractional"] ) def test_ALFFSpheres_python_vs_afni( + tmp_path: Path, fractional: bool, ) -> None: """Test ALFFSpheres python vs afni results. Parameters ---------- + tmp_path : pathlib.Path + The path to the test directory. fractional : bool Whether to compute fractional ALFF or not. + """ with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + + WorkDirManager().workdir = tmp_path marker_python = ALFFSpheres( coords=_COORDINATES, radius=5, @@ -142,12 +161,13 @@ def test_ALFFSpheres_storage( ---------- tmp_path : pathlib.Path The path to the test directory. + """ with PartlyCloudyTestingDataGrabber() as dg: # Use first subject input = dg["sub-01"] input = DefaultDataReader().fit_transform(input) - # Create ParcelAggregation object + WorkDirManager().workdir = tmp_path marker = ALFFSpheres( coords=_COORDINATES, radius=5, From fa72848bbbf74edf36afacc5dca3aaf38c737c83 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 17:37:35 +0200 Subject: [PATCH 22/30] fix: update WorkDirManager in ReHo tests --- .../markers/reho/tests/test_reho_estimator.py | 41 ++++++++++++++++--- .../markers/reho/tests/test_reho_parcels.py | 29 +++++++++++-- .../markers/reho/tests/test_reho_spheres.py | 29 +++++++++++-- 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/junifer/markers/reho/tests/test_reho_estimator.py b/junifer/markers/reho/tests/test_reho_estimator.py index 39bdca76c..9d039b0c5 100644 --- a/junifer/markers/reho/tests/test_reho_estimator.py +++ b/junifer/markers/reho/tests/test_reho_estimator.py @@ -4,6 +4,7 @@ # License: AGPL import time +from pathlib import Path import nibabel as nib import pytest @@ -11,18 +12,28 @@ from junifer.datareader.default import DefaultDataReader from junifer.markers.reho.reho_estimator import ReHoEstimator +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.testing.datagrabbers import PartlyCloudyTestingDataGrabber from junifer.utils.logging import logger -def test_reho_estimator_cache_python() -> None: - """Test that the cache works properly when using Python implementation.""" +def test_reho_estimator_cache_python(tmp_path: Path) -> None: + """Test that the cache works properly when using Python implementation. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: subject = dg["sub-01"] # Read data for subject subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Setup estimator reho_estimator = ReHoEstimator() @@ -124,13 +135,22 @@ def test_reho_estimator_cache_python() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_reho_estimator_cache_afni() -> None: - """Test that the cache works properly when using afni.""" +def test_reho_estimator_cache_afni(tmp_path: Path) -> None: + """Test that the cache works properly when using afni. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: subject = dg["sub-01"] # Read data for subject subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Setup estimator reho_estimator = ReHoEstimator() @@ -229,13 +249,22 @@ def test_reho_estimator_cache_afni() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_reho_estimator_afni_vs_python() -> None: - """Compare afni and Python implementations.""" +def test_reho_estimator_afni_vs_python(tmp_path: Path) -> None: + """Compare afni and Python implementations. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: subject = dg["sub-01"] # Read data for subject subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Setup estimator reho_estimator = ReHoEstimator() diff --git a/junifer/markers/reho/tests/test_reho_parcels.py b/junifer/markers/reho/tests/test_reho_parcels.py index 285560cff..488eda53a 100644 --- a/junifer/markers/reho/tests/test_reho_parcels.py +++ b/junifer/markers/reho/tests/test_reho_parcels.py @@ -10,6 +10,7 @@ from scipy.stats import pearsonr from junifer.markers.reho.reho_parcels import ReHoParcels +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.storage.sqlite import SQLiteFeatureStorage from junifer.testing.datagrabbers import SPMAuditoryTestingDataGrabber @@ -18,13 +19,22 @@ PARCELLATION = "Schaefer100x7" -def test_reho_parcels_computation() -> None: - """Test ReHoParcels fit-transform.""" +def test_reho_parcels_computation(tmp_path: Path) -> None: + """Test ReHoParcels fit-transform. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with SPMAuditoryTestingDataGrabber() as dg: # Use first subject subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker reho_parcels_marker = ReHoParcels(parcellation=PARCELLATION) # Fit transform marker on data @@ -49,14 +59,23 @@ def test_reho_parcels_computation() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_reho_parcels_computation_comparison() -> None: - """Test ReHoParcels fit-transform implementation comparison..""" +def test_reho_parcels_computation_comparison(tmp_path: Path) -> None: + """Test ReHoParcels fit-transform implementation comparison. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with SPMAuditoryTestingDataGrabber() as dg: # Use first subject subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker with use_afni=False reho_parcels_marker_python = ReHoParcels( parcellation=PARCELLATION, use_afni=False @@ -101,6 +120,8 @@ def test_reho_parcels_storage(tmp_path: Path) -> None: subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker reho_parcels_marker = ReHoParcels(parcellation=PARCELLATION) # Initialize storage diff --git a/junifer/markers/reho/tests/test_reho_spheres.py b/junifer/markers/reho/tests/test_reho_spheres.py index 3026bc9f5..c662980be 100644 --- a/junifer/markers/reho/tests/test_reho_spheres.py +++ b/junifer/markers/reho/tests/test_reho_spheres.py @@ -10,6 +10,7 @@ from scipy.stats import pearsonr from junifer.markers.reho.reho_spheres import ReHoSpheres +from junifer.pipeline import WorkDirManager from junifer.pipeline.utils import _check_afni from junifer.storage.sqlite import SQLiteFeatureStorage from junifer.testing.datagrabbers import SPMAuditoryTestingDataGrabber @@ -18,13 +19,22 @@ COORDINATES = "DMNBuckner" -def test_reho_spheres_computation() -> None: - """Test ReHoSpheres fit-transform.""" +def test_reho_spheres_computation(tmp_path: Path) -> None: + """Test ReHoSpheres fit-transform. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with SPMAuditoryTestingDataGrabber() as dg: # Use first subject subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker reho_spheres_marker = ReHoSpheres(coords=COORDINATES, radius=10.0) # Fit transform marker on data @@ -49,14 +59,23 @@ def test_reho_spheres_computation() -> None: @pytest.mark.skipif( _check_afni() is False, reason="requires afni to be in PATH" ) -def test_reho_spheres_computation_comparison() -> None: - """Test ReHoSpheres fit-transform implementation comparison..""" +def test_reho_spheres_computation_comparison(tmp_path: Path) -> None: + """Test ReHoSpheres fit-transform implementation comparison. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ with SPMAuditoryTestingDataGrabber() as dg: # Use first subject subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker with use_afni=False reho_spheres_marker_python = ReHoSpheres( coords=COORDINATES, radius=10.0, use_afni=False @@ -101,6 +120,8 @@ def test_reho_spheres_storage(tmp_path: Path) -> None: subject_data = dg["sub001"] # Load image to memory fmri_img = nimg.load_img(subject_data["BOLD"]["path"]) + # Update workdir to current test's tmp_path + WorkDirManager().workdir = tmp_path # Initialize marker reho_spheres_marker = ReHoSpheres(coords=COORDINATES, radius=10.0) # Initialize storage From 16fdef2ea9e476d2ebda071709aa889cc2c1c223 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 17:38:10 +0200 Subject: [PATCH 23/30] chore: add changelog 254.feature --- docs/changes/newsfragments/254.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/newsfragments/254.feature diff --git a/docs/changes/newsfragments/254.feature b/docs/changes/newsfragments/254.feature new file mode 100644 index 000000000..cdc461cb6 --- /dev/null +++ b/docs/changes/newsfragments/254.feature @@ -0,0 +1 @@ +Introduce :class:`junifer.pipeline.WorkDirManager` singleton class to manage working and temporary directories across pipeline by `Synchon Mandal`_ From 27b5a21d5da50e8eeff1a740af21161d7e4992c9 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 18:13:37 +0200 Subject: [PATCH 24/30] fix: improve tempdir cleaning in ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 5f52ef15a..f9eb741fb 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -321,8 +321,8 @@ def fit_transform( # Clear the cache self._compute.cache_clear() # Clear temporary directory files - for file_ in self.temp_dir_path.iterdir(): - file_.unlink(missing_ok=True) + WorkDirManager().delete_tempdir(self.temp_dir_path) + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") # Set the new file path self._file_path = bold_path else: From d72498b3835c9ef74c1e1d9e31c183ee1620183b Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 18:14:00 +0200 Subject: [PATCH 25/30] fix: improve tempdir cleaning in ReHoEstimator --- junifer/markers/reho/reho_estimator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 75f67ab03..676e22ab1 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -465,8 +465,8 @@ def fit_transform( # Clear the cache self._compute.cache_clear() # Clear temporary directory files - for file_ in self.temp_dir_path.iterdir(): - file_.unlink(missing_ok=True) + WorkDirManager().delete_tempdir(self.temp_dir_path) + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="reho") # Set the new file path self._file_path = bold_path else: From 56a4796ca36b90ee88124938a014f767eb81b3b5 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 10 Oct 2023 18:39:23 +0200 Subject: [PATCH 26/30] fix: update WorkDirManager tests to allow complete test suite to pass --- junifer/pipeline/tests/test_workdir_manager.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/junifer/pipeline/tests/test_workdir_manager.py b/junifer/pipeline/tests/test_workdir_manager.py index f969c9e20..7cebfe815 100644 --- a/junifer/pipeline/tests/test_workdir_manager.py +++ b/junifer/pipeline/tests/test_workdir_manager.py @@ -3,7 +3,6 @@ # Authors: Synchon Mandal # License: AGPL -import tempfile from pathlib import Path from junifer.pipeline import WorkDirManager @@ -16,14 +15,8 @@ def test_workdir_manager_singleton() -> None: assert id(workdir_mgr_1) == id(workdir_mgr_2) -def test_workdir_manager_auto_workdir() -> None: - """Test that automatic workdir creation is in temporary directory.""" - workdir_mgr = WorkDirManager() - assert workdir_mgr.workdir == Path(tempfile.gettempdir()) / "junifer" - - -def test_workdir_manager_explicit_workdir(tmp_path: Path) -> None: - """Test that WorkDirManager works correctly with an explicit workdir. +def test_workdir_manager_workdir(tmp_path: Path) -> None: + """Test WorkDirManager correctly sets workdir. Parameters ---------- @@ -37,7 +30,7 @@ def test_workdir_manager_explicit_workdir(tmp_path: Path) -> None: def test_workdir_manager_get_and_delete_tempdir(tmp_path: Path) -> None: - """Test getting and deleting temporary directories. + """Test WorkDirManager gets and deletes temporary directories correctly. Parameters ---------- From d7b778497993dda9058bd78471fa8b9632ab2052 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 11 Oct 2023 14:09:09 +0200 Subject: [PATCH 27/30] fix: update tempdir creation logic for ReHoEstimator --- junifer/markers/reho/reho_estimator.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 676e22ab1..d8a6c7e81 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -48,13 +48,13 @@ def __init__(self) -> None: self._file_path = None # Create temporary directory for intermittent storage of assets during # computation via afni's 3dReHo - self.temp_dir_path = WorkDirManager().get_tempdir(prefix="reho") + self.temp_dir_path = None def __del__(self) -> None: """Cleanup.""" # Delete temporary directory and ignore errors for read-only files - WorkDirManager().delete_tempdir(self.temp_dir_path) - WorkDirManager()._cleanup() + if self.temp_dir_path is not None: + WorkDirManager().delete_tempdir(self.temp_dir_path) def _compute_reho_afni( self, @@ -136,12 +136,15 @@ def _compute_reho_afni( https://doi.org/10.1089/brain.2013.0154 """ + # Note: self.temp_dir_path is sure to exist before proceeding, so + # types checks are ignored further on. + # Save niimg to nii.gz - nifti_in_file_path = self.temp_dir_path / "input.nii" + nifti_in_file_path = self.temp_dir_path / "input.nii" # type: ignore nib.save(data, nifti_in_file_path) # Set 3dReHo command - reho_afni_out_path_prefix = self.temp_dir_path / "reho" + reho_afni_out_path_prefix = self.temp_dir_path / "reho" # type: ignore reho_cmd: List[str] = [ "3dReHo", f"-prefix {reho_afni_out_path_prefix.resolve()}", @@ -193,7 +196,8 @@ def _compute_reho_afni( sha256_params = hashlib.sha256(bytes(reho_cmd_str, "utf-8")) # Convert afni to nifti reho_afni_to_nifti_out_path = ( - self.temp_dir_path / f"output_{sha256_params.hexdigest()}.nii" + self.temp_dir_path + / f"output_{sha256_params.hexdigest()}.nii" # type: ignore ) convert_cmd: List[str] = [ "3dAFNItoNIFTI", @@ -224,7 +228,7 @@ def _compute_reho_afni( ) # Cleanup intermediate files - for fname in self.temp_dir_path.glob("reho*"): + for fname in self.temp_dir_path.glob("reho*"): # type: ignore fname.unlink() # Load nifti @@ -404,7 +408,7 @@ def _compute_reho_python( ) output = nimg.new_img_like(data, reho_map, copy_header=False) - return output + return output # type: ignore @lru_cache(maxsize=None, typed=True) def _compute( @@ -430,6 +434,8 @@ def _compute( """ if use_afni: + # Create new temporary directory before using AFNI + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="reho") output = self._compute_reho_afni(data, **reho_params) else: output = self._compute_reho_python(data, **reho_params) @@ -465,8 +471,8 @@ def fit_transform( # Clear the cache self._compute.cache_clear() # Clear temporary directory files - WorkDirManager().delete_tempdir(self.temp_dir_path) - self.temp_dir_path = WorkDirManager().get_tempdir(prefix="reho") + if self.temp_dir_path is not None: + WorkDirManager().delete_tempdir(self.temp_dir_path) # Set the new file path self._file_path = bold_path else: From 45e36d9fea1a5f8c7e7a628d7b2dc95e91ffaadc Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 11 Oct 2023 14:09:42 +0200 Subject: [PATCH 28/30] fix: update ReHoEstimator tests --- .../markers/reho/tests/test_reho_estimator.py | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/junifer/markers/reho/tests/test_reho_estimator.py b/junifer/markers/reho/tests/test_reho_estimator.py index 9d039b0c5..a08280749 100644 --- a/junifer/markers/reho/tests/test_reho_estimator.py +++ b/junifer/markers/reho/tests/test_reho_estimator.py @@ -37,6 +37,7 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: # Setup estimator reho_estimator = ReHoEstimator() + # Compute without cache first_tic = time.time() reho_map_without_cache = reho_estimator.fit_transform( use_afni=False, @@ -48,11 +49,8 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: f"ReHo estimator in Python without cache: {first_toc - first_tic}" ) assert isinstance(reho_map_without_cache, nib.Nifti1Image) - # Count intermediate files - n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now fit again, should be faster + # Compute again with cache, should be faster second_tic = time.time() reho_map_with_cache = reho_estimator.fit_transform( use_afni=False, @@ -66,12 +64,8 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: assert isinstance(reho_map_with_cache, nib.Nifti1Image) # Check that cache is being used assert (second_toc - second_tic) < ((first_toc - first_tic) / 1000) - # Count intermediate files - n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now change a parameter, should compute again, without clearing the - # cache + # Change a parameter and compute again without cache third_tic = time.time() reho_map_with_partial_cache = reho_estimator.fit_transform( use_afni=False, @@ -85,11 +79,8 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: assert isinstance(reho_map_with_partial_cache, nib.Nifti1Image) # Should require more time assert (third_toc - third_tic) > ((first_toc - first_tic) / 10) - # Count intermediate files - n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now fit again with the previous params, should be fast + # Compute again with cache, should be faster fourth_tic = time.time() reho_map_with_new_cache = reho_estimator.fit_transform( use_afni=False, @@ -103,11 +94,8 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: assert isinstance(reho_map_with_new_cache, nib.Nifti1Image) # Should require less time assert (fourth_toc - fourth_tic) < ((first_toc - first_tic) / 1000) - # Count intermediate files - n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now change the data, it should clear the cache + # Change the data and it should clear the cache with PartlyCloudyTestingDataGrabber() as dg: subject = dg["sub-02"] # Read data for new subject @@ -127,9 +115,6 @@ def test_reho_estimator_cache_python(tmp_path: Path) -> None: assert isinstance(reho_map_with_different_cache, nib.Nifti1Image) # Should take less time assert (fifth_toc - fifth_tic) > ((first_toc - first_tic) / 10) - # Count intermediate files - n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python @pytest.mark.skipif( @@ -154,6 +139,7 @@ def test_reho_estimator_cache_afni(tmp_path: Path) -> None: # Setup estimator reho_estimator = ReHoEstimator() + # Compute without cache first_tic = time.time() reho_map_without_cache = reho_estimator.fit_transform( use_afni=True, @@ -169,7 +155,7 @@ def test_reho_estimator_cache_afni(tmp_path: Path) -> None: n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) assert n_files == 2 # input + reho - # Now fit again, should be faster + # Compute again with cache, should be faster second_tic = time.time() reho_map_with_cache = reho_estimator.fit_transform( use_afni=True, @@ -186,8 +172,7 @@ def test_reho_estimator_cache_afni(tmp_path: Path) -> None: n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) assert n_files == 2 # input + reho - # Now change a parameter, should compute again, without clearing the - # cache + # Change a parameter and compute again without cache third_tic = time.time() reho_map_with_partial_cache = reho_estimator.fit_transform( use_afni=True, @@ -203,9 +188,9 @@ def test_reho_estimator_cache_afni(tmp_path: Path) -> None: assert (third_toc - third_tic) > ((first_toc - first_tic) / 10) # Count intermediate files n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 3 # input + 2 * reho + assert n_files == 2 # input + reho - # Now fit again with the previous params, should be fast + # Compute with cache, should be faster fourth_tic = time.time() reho_map_with_new_cache = reho_estimator.fit_transform( use_afni=True, @@ -220,9 +205,9 @@ def test_reho_estimator_cache_afni(tmp_path: Path) -> None: # Should require less time assert (fourth_toc - fourth_tic) < ((first_toc - first_tic) / 1000) n_files = len(list(reho_estimator.temp_dir_path.glob("*"))) - assert n_files == 3 # input + 2 * reho + assert n_files == 2 # input + reho - # Now change the data, it should clear the cache + # Change the data and it should clear the cache with PartlyCloudyTestingDataGrabber() as dg: subject = dg["sub-02"] # Read data for new subject From 0ea1fd7451e0579dbfa2b6117f7bdc3d45adb607 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 11 Oct 2023 14:10:56 +0200 Subject: [PATCH 29/30] fix: update tempdir creation logic for ALFFEstimator --- junifer/markers/falff/falff_estimator.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index f9eb741fb..e6e5bcf56 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -50,13 +50,13 @@ def __init__(self) -> None: self._file_path = None # Create temporary directory for intermittent storage of assets during # computation via afni's 3dRSFC - self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") + self.temp_dir_path = None def __del__(self) -> None: """Cleanup.""" # Delete temporary directory and ignore errors for read-only files - WorkDirManager().delete_tempdir(self.temp_dir_path) - WorkDirManager()._cleanup() + if self.temp_dir_path is not None: + WorkDirManager().delete_tempdir(self.temp_dir_path) @staticmethod def _run_afni_cmd(cmd: str) -> None: @@ -126,6 +126,8 @@ def _compute_alff_afni( If the AFNI commands fails due to some issues """ + # Note: self.temp_dir_path is sure to exist before proceeding, so + # types checks are ignored further on. # Save niimg to nii.gz nifti_in_file_path = self.temp_dir_path / "input.nii" @@ -165,7 +167,7 @@ def _compute_alff_afni( self._run_afni_cmd(convert_cmd) # Cleanup intermediate files - for fname in self.temp_dir_path.glob("temp_*"): + for fname in self.temp_dir_path.glob("temp_*"): # type: ignore fname.unlink() # Load niftis @@ -271,6 +273,8 @@ def _compute( fALFF map. """ if use_afni: + # Create new temporary directory before using AFNI + self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") output = self._compute_alff_afni( data=data, highpass=highpass, @@ -321,8 +325,8 @@ def fit_transform( # Clear the cache self._compute.cache_clear() # Clear temporary directory files - WorkDirManager().delete_tempdir(self.temp_dir_path) - self.temp_dir_path = WorkDirManager().get_tempdir(prefix="falff") + if self.temp_dir_path is not None: + WorkDirManager().delete_tempdir(self.temp_dir_path) # Set the new file path self._file_path = bold_path else: From d2f870b6a4bc170a6729aabaa0e628894420ab96 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 11 Oct 2023 14:12:03 +0200 Subject: [PATCH 30/30] fix: update ALFFEstimator tests --- .../falff/tests/test_falff_estimator.py | 102 +++++++++--------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/junifer/markers/falff/tests/test_falff_estimator.py b/junifer/markers/falff/tests/test_falff_estimator.py index 01e39c560..dc6b338dc 100644 --- a/junifer/markers/falff/tests/test_falff_estimator.py +++ b/junifer/markers/falff/tests/test_falff_estimator.py @@ -27,17 +27,21 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: The path to the test directory. """ + # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: - input = dg["sub-01"] - - input = DefaultDataReader().fit_transform(input) - + subject = dg["sub-01"] + # Read data for subject + subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path WorkDirManager().workdir = tmp_path + # Setup estimator estimator = ALFFEstimator() + + # Compute without cache start_time = time.time() alff, falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -46,14 +50,12 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: logger.info(f"ALFF Estimator First time: {first_time}") assert isinstance(alff, Nifti1Image) assert isinstance(falff, Nifti1Image) - n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now fit again, should be faster + # Compute again with cache, should be faster start_time = time.time() alff, falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -61,15 +63,12 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: second_time = time.time() - start_time logger.info(f"ALFF Estimator Second time: {second_time}") assert second_time < (first_time / 1000) - n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now change a parameter, should compute again, without clearing the - # cache + # Change a parameter and compute again without cache start_time = time.time() alff, falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.11, tr=None, @@ -77,14 +76,12 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: third_time = time.time() - start_time logger.info(f"ALFF Estimator Third time: {third_time}") assert third_time > (first_time / 10) - n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now fit again with the previous params, should be fast + # Compute again with cache, should be faster start_time = time.time() alff, falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -92,19 +89,17 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: fourth = time.time() - start_time logger.info(f"ALFF Estimator Fourth time: {fourth}") assert fourth < (first_time / 1000) - n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python - # Now change the data, it should clear the cache + # Change the data and it should clear the cache with PartlyCloudyTestingDataGrabber() as dg: - input = dg["sub-02"] - - input = DefaultDataReader().fit_transform(input) + subject = dg["sub-02"] + # Read data for new subject + subject_data = DefaultDataReader().fit_transform(subject) start_time = time.time() alff, falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -112,8 +107,6 @@ def test_ALFFEstimator_cache_python(tmp_path: Path) -> None: fifth = time.time() - start_time logger.info(f"ALFF Estimator Fifth time: {fifth}") assert fifth > (first_time / 10) - n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 0 # no files in python @pytest.mark.skipif( @@ -128,17 +121,21 @@ def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: The path to the test directory. """ + # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: - input = dg["sub-01"] - - input = DefaultDataReader().fit_transform(input) - + subject = dg["sub-01"] + # Read data for subject + subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path WorkDirManager().workdir = tmp_path + # Setup estimator estimator = ALFFEstimator() + + # Compute with cache start_time = time.time() alff, falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -150,11 +147,11 @@ def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: n_files = len(list(estimator.temp_dir_path.glob("*"))) assert n_files == 3 # input + alff + falff - # Now fit again, should be faster + # Compute again with cache, should be faster start_time = time.time() alff, falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -165,12 +162,11 @@ def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: n_files = len(list(estimator.temp_dir_path.glob("*"))) assert n_files == 3 # input + alff + falff - # Now change a parameter, should compute again, without clearing the - # cache + # Change a parameter and compute again without cache start_time = time.time() alff, falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.11, tr=None, @@ -179,13 +175,13 @@ def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: logger.info(f"ALFF Estimator Third time: {third_time}") assert third_time > (first_time / 10) n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 5 # input + 2 * alff + 2 * falff + assert n_files == 3 # input + alff + falff - # Now fit again with the previous params, should be fast + # Compute with cache, should be faster start_time = time.time() alff, falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -194,18 +190,18 @@ def test_ALFFEstimator_cache_afni(tmp_path: Path) -> None: logger.info(f"ALFF Estimator Fourth time: {fourth}") assert fourth < (first_time / 1000) n_files = len(list(estimator.temp_dir_path.glob("*"))) - assert n_files == 5 # input + 2 * alff + 2 * falff + assert n_files == 3 # input + alff + falff - # Now change the data, it should clear the cache + # Change the data and it should clear the cache with PartlyCloudyTestingDataGrabber() as dg: - input = dg["sub-02"] - - input = DefaultDataReader().fit_transform(input) + subject = dg["sub-02"] + # Read data for new subject + subject_data = DefaultDataReader().fit_transform(subject) start_time = time.time() alff, falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=None, @@ -229,18 +225,20 @@ def test_ALFFEstimator_afni_vs_python(tmp_path: Path) -> None: The path to the test directory. """ + # Get subject from datagrabber with PartlyCloudyTestingDataGrabber() as dg: - input = dg["sub-01"] - - input = DefaultDataReader().fit_transform(input) - + subject = dg["sub-01"] + # Read data for subject + subject_data = DefaultDataReader().fit_transform(subject) + # Update workdir to current test's tmp_path WorkDirManager().workdir = tmp_path + # Setup estimator estimator = ALFFEstimator() # Use an arbitrary TR to test the AFNI vs Python implementation afni_alff, afni_falff = estimator.fit_transform( use_afni=True, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=2.5, @@ -248,7 +246,7 @@ def test_ALFFEstimator_afni_vs_python(tmp_path: Path) -> None: python_alff, python_falff = estimator.fit_transform( use_afni=False, - input_data=input["BOLD"], + input_data=subject_data["BOLD"], highpass=0.01, lowpass=0.1, tr=2.5,