diff --git a/docs/changes/newsfragments/403.enh b/docs/changes/newsfragments/403.enh new file mode 100644 index 000000000..f4f5f8589 --- /dev/null +++ b/docs/changes/newsfragments/403.enh @@ -0,0 +1 @@ +Use :func:`junifer.utils.ConfigManager` to skip time-consuming dataset ID and dirty checks for :class:`.DataladDataGrabber` by `Fede Raimondo`_ diff --git a/docs/conf.py b/docs/conf.py index 6765d35e1..e4353cd37 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -96,6 +96,7 @@ nitpick_ignore_regex = [ ("py:obj", "sqlalchemy.engine.Engine"), # ignore sqlalchemy ("py:class", "pipeline.Pipeline"), # nilearn ("py:obj", "neurokit2.*"), # ignore neurokit2 + ("py:obj", "datalad.*"), # ignore datalad ] # -- Options for HTML output ------------------------------------------------- diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index 8081c45ee..f7eb88d95 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -17,7 +17,7 @@ from datalad.support.exceptions import IncompleteResultsError from datalad.support.gitrepo import GitRepo from ..pipeline import WorkDirManager -from ..utils import logger, raise_error, warn_with_log +from ..utils import config, logger, raise_error, warn_with_log from .base import BaseDataGrabber @@ -143,27 +143,49 @@ class DataladDataGrabber(BaseDataGrabber): """ return super().datadir / self._rootdir - def _get_dataset_id_remote(self) -> str: + def _get_dataset_id_remote(self) -> tuple[str, bool]: """Get the dataset ID from the remote. Returns ------- str The dataset ID. + bool + Whether the dataset is dirty. + + Raises + ------ + ValueError + If the dataset ID cannot be obtained from the remote. """ remote_id = None + is_dirty = False with tempfile.TemporaryDirectory() as tmpdir: - logger.debug(f"Querying {self.uri} for dataset ID") - repo = GitRepo.clone( - self.uri, path=tmpdir, clone_options=["-n", "--depth=1"] + if not config.get("datagrabber.skipidcheck", False): + logger.debug(f"Querying {self.uri} for dataset ID") + repo = GitRepo.clone( + self.uri, path=tmpdir, clone_options=["-n", "--depth=1"] + ) + repo.checkout(name=".datalad/config", options=["HEAD"]) + remote_id = repo.config.get("datalad.dataset.id", None) + logger.debug(f"Got remote dataset ID = {remote_id}") + + if not config.get("datagrabber.skipdirtycheck", False): + is_dirty = repo.dirty + else: + logger.debug("Skipping dirty check") + is_dirty = False + else: + logger.debug("Skipping dataset ID check") + remote_id = self._dataset.id + is_dirty = False + logger.debug( + f"Remote dataset is {'' if is_dirty else 'not'} dirty" ) - repo.checkout(name=".datalad/config", options=["HEAD"]) - remote_id = repo.config.get("datalad.dataset.id", None) - logger.debug(f"Got remote dataset ID = {remote_id}") if remote_id is None: raise_error("Could not get dataset ID from remote") - return remote_id + return remote_id, is_dirty def _dataset_get(self, out: dict) -> dict: """Get the dataset found from the path in ``out``. @@ -178,6 +200,11 @@ class DataladDataGrabber(BaseDataGrabber): dict The unmodified input dictionary. + Raises + ------ + datalad.support.exceptions.IncompleteResultsError + If there is a datalad-related problem while fetching data. + """ to_get = [] for type_val in out.values(): @@ -230,6 +257,8 @@ class DataladDataGrabber(BaseDataGrabber): ------ ValueError If the dataset is already installed but with a different ID. + datalad.support.exceptions.IncompleteResultsError + If there is a datalad-related problem while cloning dataset. """ isinstalled = dl.Dataset(self._datadir).is_installed() @@ -238,23 +267,23 @@ class DataladDataGrabber(BaseDataGrabber): self._got_files = [] self._dataset: dl.Dataset = dl.Dataset(self._datadir) - remote_id = self._get_dataset_id_remote() + # Check if dataset is already installed with a different ID + remote_id, is_dirty = self._get_dataset_id_remote() if remote_id != self._dataset.id: raise_error( "Dataset already installed but with a different " f"ID: {self._dataset.id} (local) != {remote_id} (remote)" ) - # Check for dirty datasets: - status = self._dataset.status() - if any(x["state"] != "clean" for x in status): - self.datalad_dirty = True + # Conditional reporting on dataset dirtiness + self.datalad_dirty = is_dirty + if self.datalad_dirty: warn_with_log( - "At least one file is not clean, Junifer will " - "consider this dataset as dirty." + "At least one file is not clean, " + f"marking dataset (id: {self._dataset.id}) as dirty." ) else: - logger.debug("Dataset is clean") + logger.debug(f"Dataset (id: {self._dataset.id}) is clean") else: logger.debug(f"Installing dataset {self.uri} to {self._datadir}") diff --git a/junifer/datagrabber/tests/test_datalad_base.py b/junifer/datagrabber/tests/test_datalad_base.py index 02a94758f..f3e5e4661 100644 --- a/junifer/datagrabber/tests/test_datalad_base.py +++ b/junifer/datagrabber/tests/test_datalad_base.py @@ -3,12 +3,14 @@ # Authors: Synchon Mandal # License: AGPL +import warnings from pathlib import Path import datalad.api as dl import pytest from junifer.datagrabber import DataladDataGrabber +from junifer.utils import config _testing_dataset = { @@ -94,6 +96,12 @@ def test_DataladDataGrabber_install_errors( with pytest.raises(ValueError, match=r"different ID"): with dg: pass + # Set config to skip id check and test + config.set(key="datagrabber.skipidcheck", val=True) + with dg: + pass + # Reset config + config.delete("datagrabber.skipidcheck") elem1_t1w = datadir / "example_bids/sub-01/anat/sub-01_T1w.nii.gz" elem1_t1w.unlink() @@ -104,6 +112,14 @@ def test_DataladDataGrabber_install_errors( with pytest.warns(RuntimeWarning, match=r"one file is not clean"): with dg: pass + # Set config to skip dirty check and test + with warnings.catch_warnings(): + warnings.simplefilter("error") + config.set(key="datagrabber.skipdirtycheck", val=True) + with dg: + pass + # Reset config + config.delete("datagrabber.skipdirtycheck") def test_DataladDataGrabber_clone_cleanup( @@ -246,7 +262,7 @@ def test_DataladDataGrabber_previously_cloned( meta = elem1["BOLD"]["meta"] assert "datagrabber" in meta assert "datalad_dirty" in meta["datagrabber"] - assert meta["datagrabber"]["datalad_dirty"] is False + assert meta["datagrabber"]["datalad_dirty"] is True assert "datalad_commit_id" in meta["datagrabber"] assert meta["datagrabber"]["datalad_commit_id"] == commit assert "datalad_id" in meta["datagrabber"] @@ -326,7 +342,7 @@ def test_DataladDataGrabber_previously_cloned_and_get( meta = elem1["BOLD"]["meta"] assert "datagrabber" in meta assert "datalad_dirty" in meta["datagrabber"] - assert meta["datagrabber"]["datalad_dirty"] is False + assert meta["datagrabber"]["datalad_dirty"] is True assert "datalad_commit_id" in meta["datagrabber"] assert meta["datagrabber"]["datalad_commit_id"] == commit assert "datalad_id" in meta["datagrabber"]