diff --git a/docs/changes/latest.inc b/docs/changes/latest.inc index ce876f5ee..fb319fc0b 100644 --- a/docs/changes/latest.inc +++ b/docs/changes/latest.inc @@ -71,6 +71,8 @@ Bugs - Fix a bug in which :func:`junifer.stats.count` will not be correctly applied across an axis (:gh:`195` by `Fede Raimondo`_). +- Fix an issue with datalad cache and locks in which the overriden settings in Junifer were not propagated to subprocesses, resulting in using the default settings (:gh:`199` by `Fede Raimondo`_). + API changes ~~~~~~~~~~~ diff --git a/junifer/api/cli.py b/junifer/api/cli.py index fe5288f4e..c24482b8c 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -74,6 +74,45 @@ def _parse_elements(element: str, config: Dict) -> Union[List, None]: return elements +def _validate_verbose( + ctx: click.Context, param: str, value: str +) -> Union[str, int]: + """Validate verbose option. + + Parameters + ---------- + ctx : click.Context + The context of the command. + param : str + The parameter to validate. + value : str + The value to validate. + + Returns + ------- + str or int + The validated value. + """ + if isinstance(value, int): + return value + + valid_strings = ["error", "warning", "info", "debug"] + if isinstance(value, str) and value.lower() in valid_strings: + return value.upper() + + try: + value = int(value) # type: ignore + return value + except ValueError: + # If we get here, the value is not a valid integer. + pass + + # If we get here, the value is not valid. + raise click.BadParameter( + f"verbose must be one of {valid_strings} or an integer" + ) + + @click.group() def cli() -> None: # pragma: no cover """CLI for JUelich NeuroImaging FEature extractoR.""" @@ -90,10 +129,11 @@ def cli() -> None: # pragma: no cover @click.option( "-v", "--verbose", - type=click.Choice(["warning", "info", "debug"], case_sensitive=False), + type=click.UNPROCESSED, + callback=_validate_verbose, default="info", ) -def run(filepath: click.Path, element: str, verbose: click.Choice) -> None: +def run(filepath: click.Path, element: str, verbose: Union[str, int]) -> None: """Run command for CLI. \f Parameters @@ -106,7 +146,7 @@ def run(filepath: click.Path, element: str, verbose: click.Choice) -> None: The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=str(verbose).upper()) + configure_logging(level=verbose) # TODO: add validation config = parse_yaml(filepath) # type: ignore workdir = config["workdir"] @@ -136,10 +176,11 @@ def run(filepath: click.Path, element: str, verbose: click.Choice) -> None: @click.option( "-v", "--verbose", - type=click.Choice(["warning", "info", "debug"], case_sensitive=False), + type=click.UNPROCESSED, + callback=_validate_verbose, default="info", ) -def collect(filepath: click.Path, verbose: click.Choice) -> None: +def collect(filepath: click.Path, verbose: Union[str, int]) -> None: """Collect command for CLI. \f Parameters @@ -150,7 +191,7 @@ def collect(filepath: click.Path, verbose: click.Choice) -> None: The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=str(verbose).upper()) + configure_logging(level=verbose) # TODO: add validation config = parse_yaml(filepath) # type: ignore storage = config["storage"] @@ -171,7 +212,8 @@ def collect(filepath: click.Path, verbose: click.Choice) -> None: @click.option( "-v", "--verbose", - type=click.Choice(["warning", "info", "debug"], case_sensitive=False), + type=click.UNPROCESSED, + callback=_validate_verbose, default="info", ) def queue( @@ -179,7 +221,7 @@ def queue( element: str, overwrite: bool, submit: bool, - verbose: click.Choice, + verbose: Union[str, int], ) -> None: """Queue command for CLI. \f @@ -197,7 +239,7 @@ def queue( The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=str(verbose).upper()) + configure_logging(level=verbose) # TODO: add validation config = parse_yaml(filepath) # type: ignore elements = _parse_elements(element, config) diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index 7713a3f5a..d6ad0e16f 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -6,6 +6,7 @@ # License: AGPL import atexit +import os import shutil import tempfile from pathlib import Path @@ -13,6 +14,7 @@ from typing import Dict, Optional, Tuple, Union import datalad import datalad.api as dl +from datalad.support.exceptions import IncompleteResultsError from datalad.support.gitrepo import GitRepo from ..api.decorators import register_datagrabber @@ -82,21 +84,10 @@ class DataladDataGrabber(BaseDataGrabber): sockets_dir.mkdir(parents=True, exist_ok=False) locks_dir.mkdir(parents=True, exist_ok=False) logger.debug(f"Setting datalad cache to {cache_dir}") - datalad.cfg.set( - "datalad.locations.cache", - cache_dir.as_posix(), - scope="override", - ) - datalad.cfg.set( - "datalad.locations.sockets", - sockets_dir.as_posix(), - scope="override", - ) - datalad.cfg.set( - "datalad.locations.locks", - locks_dir.as_posix(), - scope="override", - ) + os.environ["DATALAD_LOCATIONS_CACHE"] = cache_dir.as_posix() + os.environ["DATALAD_LOCATIONS_SOCKETS"] = sockets_dir.as_posix() + os.environ["DATALAD_LOCATIONS_LOCKS"] = locks_dir.as_posix() + datalad.cfg.reload() logger.debug( "Datalad cache set to " f"{datalad.cfg.get('datalad.locations.cache')}" @@ -184,7 +175,12 @@ class DataladDataGrabber(BaseDataGrabber): for fname in to_get: logger.debug(f"\t: {fname}") - dl_out = self._dataset.get(to_get, result_renderer="disabled") + try: + dl_out = self._dataset.get(to_get, result_renderer="disabled") + except IncompleteResultsError as e: + raise_error( + f"Failed to get from dataset: {e.failed}" + ) if not self._was_cloned: # If the dataset was already installed, check that the # file was actually downloaded to avoid removing a diff --git a/junifer/datagrabber/tests/test_datalad_base.py b/junifer/datagrabber/tests/test_datalad_base.py index 4eee066b8..5da6eaa17 100644 --- a/junifer/datagrabber/tests/test_datalad_base.py +++ b/junifer/datagrabber/tests/test_datalad_base.py @@ -160,6 +160,49 @@ def test_datalad_clone_cleanup( assert len(list(datadir.glob("*"))) == 0 +def test_datalad_clone_create_cleanup(concrete_datagrabber: Type) -> None: + """Test datalad base tempdir clone and remove. + + Parameters + ---------- + concrete_datagrabber : DataladDataGrabber + A concrete datagrabber class to use. + """ + + # Clone whole dataset + uri = _testing_dataset["example_bids"]["uri"] + with concrete_datagrabber(datadir=None, uri=uri) as dg: + datadir = dg._tmpdir / "datadir" + elem1_bold = ( + datadir / "example_bids/sub-01/func/sub-01_task-rest_bold.nii.gz" + ) + elem1_t1w = datadir / "example_bids/sub-01/anat/sub-01_T1w.nii.gz" + + assert elem1_bold.is_file() is False + assert elem1_t1w.is_file() is False + assert datadir.exists() is True + assert dg._was_cloned is True + assert elem1_bold.is_file() is False + assert elem1_bold.is_symlink() is True + assert elem1_t1w.is_file() is False + assert elem1_t1w.is_symlink() is True + elem1 = dg["sub-01"] + assert "meta" in elem1["BOLD"] + meta = elem1["BOLD"]["meta"] + assert "datagrabber" in meta + assert "datalad_dirty" in meta["datagrabber"] + assert meta["datagrabber"]["datalad_dirty"] is False + assert hasattr(dg, "_got_files") is False + assert datadir.exists() is True + assert elem1_bold.is_file() is True + assert elem1_bold.is_symlink() is True + assert elem1_t1w.is_file() is True + assert elem1_t1w.is_symlink() is True + + assert datadir.exists() is False + assert len(list(datadir.glob("*"))) == 0 + + def test_datalad_previously_cloned( tmp_path: Path, concrete_datagrabber: Type ) -> None: