Skip to content

Commit

Permalink
Review DataladDataGrabber
Browse files Browse the repository at this point in the history
This patch just adds a few comments to consider and changes the use of
`datalad install` to `datalad clone`.
  • Loading branch information
bpoldrack committed Sep 13, 2022
1 parent becde30 commit f6cd0a4
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
3 changes: 2 additions & 1 deletion docs/changes/contributors.inc
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
.. _Kaustubh Patil: https://github.com/kaurao
.. _Leonard Sasse: https://github.com/LeSasse
.. _Amir Omidvarnia: https://github.com/omidvarnia
.. _Synchon Mandal: https://github.com/synchon
.. _Synchon Mandal: https://github.com/synchon
.. _Benjamin Poldrack: https://github.com/bpoldrack
2 changes: 2 additions & 0 deletions docs/changes/latest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ Enhancements
- Implemented SPM Auditory testing datagrabber X (:gh:`52` by `Fede Raimondo`_).

- Created a the repository based on the mockup by by `Fede Raimondo`_.
- Added comments to datalad grabber and changed to use datalad-clone instead of
datalad-install (:gh: `55` by `Benjamin Poldrack`_).

Bugs
~~~~
Expand Down
11 changes: 9 additions & 2 deletions junifer/datagrabber/datalad_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ def _dataset_get(self, out: Dict) -> Dict:
for _, v in out.items():
if "path" in v:
logger.debug(f"Getting {v['path']}")
# Note, that `self._dataset.get` without an option would get
# the content of all files in a (sub-dataset) if `v["path"]`
# was to point to a subdataset rather than a file. This may be
# a source of confusion (+ performance/storage issue) when
# implementing a grabber.
self._dataset.get(v["path"])
logger.debug("Get done")

Expand All @@ -119,13 +124,15 @@ def _dataset_get(self, out: Dict) -> Dict:
def install(self) -> None:
"""Install the datalad dataset into the datadir."""
logger.debug(f"Installing dataset {self.uri} to {self._datadir}")
self._dataset = dl.install( # type: ignore because of datalad
self._datadir, source=self.uri
self._dataset: dl.Dataset = dl.clone(
self.uri, self._datadir
)
logger.debug("Dataset installed")

def remove(self):
"""Remove the datalad dataset from the datadir."""
# This probably wants to use `reckless='kill'` or similar.
# See issue #53
self._dataset.remove(recursive=True)

def __getitem__(self, element: Union[str, Tuple]) -> Dict[str, Path]:
Expand Down

0 comments on commit f6cd0a4

Please # to comment.