From 805eb54009aa09f4aea2c94710c751057bbdc89e Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Mon, 25 Nov 2024 18:48:36 +0100 Subject: [PATCH 1/7] Use config to skip dataset id and dirty checks (time consuming) --- junifer/datagrabber/datalad_base.py | 47 +++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index 8081c45ee..e33eeed01 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``. @@ -238,7 +260,7 @@ class DataladDataGrabber(BaseDataGrabber): self._got_files = [] self._dataset: dl.Dataset = dl.Dataset(self._datadir) - remote_id = self._get_dataset_id_remote() + remote_id, is_dirty = self._get_dataset_id_remote() if remote_id != self._dataset.id: raise_error( "Dataset already installed but with a different " @@ -246,9 +268,8 @@ class DataladDataGrabber(BaseDataGrabber): ) # Check for dirty datasets: - status = self._dataset.status() - if any(x["state"] != "clean" for x in status): - self.datalad_dirty = True + 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." -- 2.52.0 From 17929ced237d2c7dec9d14cd136bc3c0d9873d92 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 12:43:26 +0100 Subject: [PATCH 2/7] chore: improve docstrings in DataladDataGrabber --- junifer/datagrabber/datalad_base.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index e33eeed01..b8d61a17e 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -200,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(): @@ -252,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() @@ -260,6 +267,7 @@ class DataladDataGrabber(BaseDataGrabber): self._got_files = [] self._dataset: dl.Dataset = dl.Dataset(self._datadir) + # 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( -- 2.52.0 From 0f2ee4f407385502569d7b01c8e05e9d3720273d Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 12:44:22 +0100 Subject: [PATCH 3/7] chore: improve warning and log messages in DataladDataGrabber --- junifer/datagrabber/datalad_base.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index b8d61a17e..f7eb88d95 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -275,15 +275,15 @@ class DataladDataGrabber(BaseDataGrabber): f"ID: {self._dataset.id} (local) != {remote_id} (remote)" ) - # Check for dirty datasets: + # 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}") -- 2.52.0 From 6711bc165c5142d10df27637e7b7d72d3815470b Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 12:44:49 +0100 Subject: [PATCH 4/7] update: config setting tests for DataladDataGrabber.install --- junifer/datagrabber/tests/test_datalad_base.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/junifer/datagrabber/tests/test_datalad_base.py b/junifer/datagrabber/tests/test_datalad_base.py index 02a94758f..f49e7a435 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( -- 2.52.0 From 69399b4cc2ff7227cc9a690f6d4a3b0b4088899c Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 12:56:14 +0100 Subject: [PATCH 5/7] chore: add changelog 403.enh --- docs/changes/newsfragments/403.enh | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/newsfragments/403.enh 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`_ -- 2.52.0 From dc4a3e136d30e1ed4f714c63e89e998bab2a2da9 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 12:56:37 +0100 Subject: [PATCH 6/7] docs: add datalad to nitpick_ignore --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) 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 ------------------------------------------------- -- 2.52.0 From 3dd47fc3ee93e4fa5edf9abc969c56f249955c91 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 27 Nov 2024 16:37:54 +0100 Subject: [PATCH 7/7] chore: fix DataladDataGrabber tests --- junifer/datagrabber/tests/test_datalad_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/junifer/datagrabber/tests/test_datalad_base.py b/junifer/datagrabber/tests/test_datalad_base.py index f49e7a435..f3e5e4661 100644 --- a/junifer/datagrabber/tests/test_datalad_base.py +++ b/junifer/datagrabber/tests/test_datalad_base.py @@ -262,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"] @@ -342,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"] -- 2.52.0