From b4b53e7822081c487a2cc654144048e05ce92f12 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 16 Mar 2022 14:39:27 +0100 Subject: [PATCH 01/40] Fixed tests, maybe it's ready to run? --- junifer/data/atlases.py | 2 +- junifer/markers/parcel.py | 4 ++-- junifer/markers/tests/test_base_marker.py | 4 ++-- junifer/markers/tests/test_collection.py | 27 ++++++++++++++++++----- junifer/testing.py | 2 +- 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/junifer/data/atlases.py b/junifer/data/atlases.py index 33cd4747f..519cbced5 100644 --- a/junifer/data/atlases.py +++ b/junifer/data/atlases.py @@ -293,7 +293,7 @@ def _retrieve_schaefer(atlas_dir, resolution, n_rois=None, yeo_networks=7): 'At least one of the atlas files is missing. ' 'Fetching using nilearn.') datasets.fetch_atlas_schaefer_2018( - n_rois=n_rois, + n_rois=n_rois, # type: ignore yeo_networks=yeo_networks, resolution_mm=resolution, data_dir=atlas_dir.as_posix()) diff --git a/junifer/markers/parcel.py b/junifer/markers/parcel.py index 79e23d405..6ede62d7e 100644 --- a/junifer/markers/parcel.py +++ b/junifer/markers/parcel.py @@ -22,13 +22,13 @@ class ParcelAggregation(BaseMarker): self.method_params = {} if method_params is None else method_params def get_output_kind(self, input): - if input in ['GMD_GM', 'GMD_WM']: + if input in ['VBM_GM', 'VBM_WM']: return 'table' if input in ['BOLD']: return 'timeseries' def store(self, kind, out, storage): - if kind in ['GMD_GM', 'GMD_WM']: + if kind in ['VBM_GM', 'VBM_WM']: storage.store_table(**out) if kind in ['BOLD']: storage.store_timeseries(**out) diff --git a/junifer/markers/tests/test_base_marker.py b/junifer/markers/tests/test_base_marker.py index cf9ed5298..f661af6df 100644 --- a/junifer/markers/tests/test_base_marker.py +++ b/junifer/markers/tests/test_base_marker.py @@ -20,13 +20,13 @@ def test_meta(): base = BaseMarker(on=['bold', 'dwi']) - t_meta = base.get_meta() + t_meta = base.get_meta('bold') assert t_meta['marker']['class'] == 'BaseMarker' assert t_meta['marker']['name'] == 'BaseMarker' base = BaseMarker(on=['bold', 'dwi'], name='mymarker') - t_meta = base.get_meta() + t_meta = base.get_meta('dwi') assert t_meta['marker']['name'] == 'mymarker' diff --git a/junifer/markers/tests/test_collection.py b/junifer/markers/tests/test_collection.py index b68d2a7fd..969f356e1 100644 --- a/junifer/markers/tests/test_collection.py +++ b/junifer/markers/tests/test_collection.py @@ -99,8 +99,8 @@ def test_MarkerCollection_storage(): # Test storage dg = OasisVBMTestingDatagrabber() with tempfile.TemporaryDirectory() as tmpdir: - uri = f'sqlite:///{tmpdir}/test.db' - storage = SQLiteFeatureStorage(uri=uri) + uri = f'{tmpdir}/test.db' + storage = SQLiteFeatureStorage(uri=uri, single_output=True) mc = MarkerCollection( markers=markers, storage=storage, datareader=DefaultDataReader()) mc.validate(dg) @@ -120,7 +120,24 @@ def test_MarkerCollection_storage(): out = mc2.fit(input) features = storage.list_features() - assert len(features) == 1 - feature_md5 = features.keys()[0] + assert len(features) == 3 + feature_md5 = list(features.keys())[0] t_feature = storage.read_df(feature_md5=feature_md5) - + fname = 'gmd_schaefer100x7_mean' + t_data = out[fname]['VBM_GM']['data'] # type: ignore + cols = out[fname]['VBM_GM']['columns'] # type: ignore + assert_array_equal(t_feature[cols].values, t_data) # type: ignore + + feature_md5 = list(features.keys())[1] + t_feature = storage.read_df(feature_md5=feature_md5) + fname = 'gmd_schaefer100x7_std' + t_data = out[fname]['VBM_GM']['data'] # type: ignore + cols = out[fname]['VBM_GM']['columns'] # type: ignore + assert_array_equal(t_feature[cols].values, t_data) # type: ignore + + feature_md5 = list(features.keys())[2] + t_feature = storage.read_df(feature_md5=feature_md5) + fname = 'gmd_schaefer100x7_trim_mean90' + t_data = out[fname]['VBM_GM']['data'] # type: ignore + cols = out[fname]['VBM_GM']['columns'] # type: ignore + assert_array_equal(t_feature[cols].values, t_data) # type: ignore diff --git a/junifer/testing.py b/junifer/testing.py index 1a20c8695..999f58f69 100644 --- a/junifer/testing.py +++ b/junifer/testing.py @@ -29,7 +29,7 @@ class OasisVBMTestingDatagrabber(BaseDataGrabber): def __getitem__(self, element): out = {} out['VBM_GM'] = self._dataset.gray_matter_maps[element - 1] - out['meta'] = {'subject': element} + out['meta'] = {'element': {'subject': element}} return out def __enter__(self): -- 2.52.0 From 0d66f7015e4ef468431a9421b27de816d5175c3a Mon Sep 17 00:00:00 2001 From: Vera Komeyer Date: Wed, 16 Mar 2022 17:02:26 +0100 Subject: [PATCH 02/40] Add retrieval of Tian atlases --- junifer/data/atlases.py | 160 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 159 insertions(+), 1 deletion(-) diff --git a/junifer/data/atlases.py b/junifer/data/atlases.py index 33cd4747f..a45fa9943 100644 --- a/junifer/data/atlases.py +++ b/junifer/data/atlases.py @@ -3,7 +3,11 @@ # License: AGPL from pathlib import Path import io +import os import requests +import wget +import shutil +import zipfile import numpy as np import pandas as pd @@ -44,6 +48,36 @@ for n_rois in range(100, 1001, 100): 'yeo_networks': t_net, } +for scale in range(1, 5): + for field in ['3T', '7T']: + if field == '7T': + space = 'MNI6thgeneration' + t_name = f'Tian{scale}x{field}x{space}' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': field, + 'valid_resolutions': [1.6] + } + else: + for space in ['MNI6thgeneration', 'MNInonlinear2009cAsym']: + if space == 'MNI6thgeneration': + t_name = f'Tian{scale}x{field}x{space}' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': field, + 'valid_resolutions': [1, 2] + } + elif space == 'MNInonlinear2009cAsym': + t_name = f'Tian{scale}x{field}x{space}' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': field, + 'valid_resolutions': [2] + } + def register_atlas(name, atlas_path, atl_labels, overwrite=False): """Register a custom user atlas. @@ -136,7 +170,15 @@ def load_atlas(name, atlas_dir=None, resolution=None, path_only=False): Number of yeo networks to use. Valid values: 7, 17. Defaults to 7. Tian : - # TODO add + scale (required) : int + Scale of atlas between 1 and 4 (defines granularity) + space (optional) : str + Space of atlas can be either 'MNI6thgeneration' or + 'MNInonlinear2009cAsym' (for some cases). + Defaults to 'MNI6thgeneration'. (For more information see + https://github.com/yetianmed/subcortex) + magneticfield (optional) : str + Options are 3T and 7T, defaults to 3T. SUIT : space (optional) : str @@ -207,6 +249,16 @@ def _retrieve_atlas(family, atlas_dir=None, resolution=None, **kwargs): (included) in steps of 100. yeo_network (optional) : int Number of yeo networks to use [7 or 17]. Defaults to 7. + Tian : + scale (required) : int + Scale of atlas between 1 and 4 (defines granularity) + space (optional) : str + Space of atlas can be either 'MNI6thgeneration' or + 'MNInonlinear2009cAsym' (for some cases). + Defaults to 'MNI6thgeneration'. (For more information see + https://github.com/yetianmed/subcortex) + magneticfield (optional) : str + Options are 3T and 7T, defaults to 3T. SUIT : space (optional) : str Space of atlas can be either 'MNI' or 'SUIT' (for more information @@ -235,6 +287,9 @@ def _retrieve_atlas(family, atlas_dir=None, resolution=None, **kwargs): elif family == 'SUIT': atlas_fname, atl_labels = \ _retrieve_suit(atlas_dir, resolution=resolution, **kwargs) + elif family == 'Tian': + atlas_fname, atl_labels = \ + _retrieve_tian(atlas_dir, resolution=resolution, **kwargs) else: raise_error( f"The provided atlas name {family} cannot be retrieved. ") @@ -312,6 +367,109 @@ def _retrieve_schaefer(atlas_dir, resolution, n_rois=None, yeo_networks=7): return atlas_fname, labels +def _retrieve_tian( + atlas_dir, resolution, scale=None, space='MNI6thgeneration', + magneticfield='3T'): + + # check validity of atlas parameters + _valid_scales = [1, 2, 3, 4] + _valid_fields = ['3T', '7T'] + if scale not in _valid_scales: + raise_error( + f'The parameter `scale` ({scale}) needs to be one of the ' + f'following: {_valid_scales}') + if field not in _valid_fields: + raise_error( + f'The parameter `magneticfield` ({field}) needs to be one of ' + f'the following: {_valid_fields}') + + if magneticfield == '3T': + _valid_spaces = ['MNI6thgeneration', 'MNInonlinear2009cAsym'] + if space == 'MNI6thgeneration': + _valid_resolutions = [1, 2] + elif space == 'MNInonlinear2009cAsym': + _valid_resolutions = [2] + if magneticfield == '7T': + _valid_spaces = ['MNI6thgeneration'] + _valid_resolutions = [1.6] + if space not in _valid_spaces: + raise_error( + f'The parameter `space` ({space}) needs to be one of ' + f'the following: {_valid_spaces}') + + resolution = _closest_resolution(resolution, _valid_resolutions) + + # show atlas parameters to user + logger.info('Atlas parameters:') + logger.info(f'\tscale: {scale}') + logger.info(f'\tspace: {space}') + logger.info(f'\tmagneticfield: {magneticfield}') + logger.info(f'\tresolution: {resolution}') + + # define file names + if magneticfield == '3T': + atlas_fname_base_3T = ( + atlas_dir / 'Tian2020MSA_v1.1' / '3T' / 'Subcortex-Only') + atlas_lname = atlas_fname_base_3T / ( + f'Tian_Subcortex_S{scale}_3T_label.txt') + if space == 'MNI6thgeneration': + atlas_fname = atlas_fname_base_3T / ( + f'Tian_Subcortex_S{scale}_{magneticfield}.nii.gz') + if resolution == 1: + atlas_fname = atlas_fname_base_3T / ( + f'Tian_Subcortex_S{scale}_{magneticfield}_{resolution}' + 'mm.nii.gz') + elif space == 'MNInonlinear2009cAsym': + space = '2009cAsym' + atlas_fname = atlas_fname_base_3T / ( + f'Tian_Subcortex_S{scale}_{magneticfield}_{space}.nii.gz') + elif magneticfield == '7T': + atlas_fname_base_7T = ( + atlas_dir / 'Tian2020MSA_v1.1' / '7T') + atlas_fname = atlas_dir / 'Tian2020MSA_v1.1' / f'{magneticfield}' / ( + f'Tian_Subcortex_S{scale}_{magneticfield}.nii.gz') + # define 7T labels (b/c currently no labels file available for 7T) + scale7Trois = {1: 16, 2: 34, 3: 54, 4: 62} + labels = [ + ('parcel_' + str(x)) for x in np.arange(1, scale7Trois[scale]+1)] + atlas_lname = atlas_fname_base_7T / ( + f'Tian_Subcortex_S{scale}_7T_labelnumbering.txt') + with open(atlas_lname, 'w') as filehandle: + for listitem in labels: + filehandle.write('%s\n' % listitem) + logger.info( + 'Currently there are no labels provided for the 7T Tian atlas. A ' + 'simple numbering scheme for distinction was therefore used.') + + # check existance of atlas + if not (atlas_fname.exists() and atlas_lname.exists()): + logger.info( + 'At least one of the atlas files is missing. ' + 'Fetching.') + + url_basis = ( + 'https://www.nitrc.org/frs/download.php/12012/Tian2020MSA_v1.1.zip') + + logger.info(f'Downloading {url_basis}') + atlas_download_dir = wget.download(url_basis, atlas_dir.as_posix()) + with zipfile.ZipFile(atlas_download_dir, 'r') as zip_ref: + zip_ref.extractall(atlas_dir.as_posix()) + # clean after unzipping + if os.path.exists(atlas_download_dir): + os.remove(atlas_download_dir) + if os.path.exists((atlas_dir / '__MACOSX').as_posix()): + shutil.rmtree((atlas_dir / '__MACOSX').as_posix()) + + labels = pd.read_csv(atlas_lname, sep=" ", header=None)[0].to_list() + + if not (atlas_fname.exists() and atlas_lname.exists()): + raise_error('There was a problem fetching the atlases.') + + labels = pd.read_csv(atlas_lname, sep=" ", header=None)[0].to_list() + + return atlas_fname, labels + + def _retrieve_suit(atlas_path, resolution, space='MNI'): logger.info('Atlas parameters:') logger.info(f'\tspace: {space}') -- 2.52.0 From b49c701588fe7a41ab2d5f9927563199119fb032 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 10:50:29 +0100 Subject: [PATCH 03/40] CLI RUN + COLLECT: missing tests on collect --- .gitignore | 3 +- examples/norun_hcpfc_pearson.py | 26 ++++--- examples/run_run_gmd_mean.py | 17 +++-- examples/yamls/gmd_mean.yaml | 25 ++++++ junifer/api/__init__.py | 3 +- junifer/api/cli.py | 49 ++++++++++++ junifer/api/parser.py | 23 ++++++ junifer/api/run.py | 44 ++++++----- junifer/markers/tests/test_collection.py | 2 +- junifer/storage/base.py | 53 ++++++------- junifer/storage/sqlite.py | 85 ++++++++++++++++----- junifer/storage/tests/test_base.py | 96 ++++++++++++++++++------ junifer/storage/tests/test_sqlite.py | 29 ++++--- junifer/testing/__init__.py | 3 + junifer/testing/datagrabbers.py | 29 +++++++ junifer/testing/registry.py | 6 ++ requirements.txt | 3 +- setup.py | 8 +- 18 files changed, 380 insertions(+), 124 deletions(-) create mode 100644 examples/yamls/gmd_mean.yaml create mode 100644 junifer/api/cli.py create mode 100644 junifer/api/parser.py create mode 100644 junifer/testing/__init__.py create mode 100644 junifer/testing/datagrabbers.py create mode 100644 junifer/testing/registry.py diff --git a/.gitignore b/.gitignore index 09d12056a..35e6aacdc 100644 --- a/.gitignore +++ b/.gitignore @@ -132,4 +132,5 @@ cython_debug/ # OS Stuff .DS_store -junifer/_version.py \ No newline at end of file +junifer/_version.py +scratch/ \ No newline at end of file diff --git a/examples/norun_hcpfc_pearson.py b/examples/norun_hcpfc_pearson.py index d2d17640d..fe2632e92 100644 --- a/examples/norun_hcpfc_pearson.py +++ b/examples/norun_hcpfc_pearson.py @@ -6,7 +6,14 @@ License: BSD 3 clause """ -from junifer.api import run_pipeline +from junifer.api import run + +datagrabber = { + 'kind': 'HCPOpenAccess', + 'modality': 'fMRI', + 'preprocessed': 'ICA+FIX', + 'space': 'volumetric', +} custom_confound_strategy = { 'filter': 'butterworth', @@ -48,18 +55,15 @@ markers = [ 'confound_strategy': custom_confound_strategy} ] -dg_params = { - 'modality': 'fMRI', - 'preprocessed': 'ICA+FIX', - 'space': 'volumetric', +storage = { + 'kind': 'SQLiteFeatureStorage', + 'uri': '/data/project/juniferexample' } -run_pipeline( +run( workdir='/tmp', - datagrabber='HCPOpenAccess', - datagrabber_params=dg_params, - element=('100408', 'REST1', "LR"), + datagrabber=datagrabber, + elements=[('100408', 'REST1', "LR")], markers=markers, - storage='SQLDataFrameStorage', - storage_params={'outpath': '/data/project/juniferexample'}, + storage=storage, ) diff --git a/examples/run_run_gmd_mean.py b/examples/run_run_gmd_mean.py index db461246d..6259530b7 100644 --- a/examples/run_run_gmd_mean.py +++ b/examples/run_run_gmd_mean.py @@ -9,10 +9,11 @@ License: BSD 3 clause import tempfile from junifer.api import run -from junifer.testing import register_testing +import junifer.testing.registry # noqa: F401 - -register_testing() +datagrabber = { + 'kind': 'OasisVBMTestingDatagrabber', +} markers = [ {'name': 'Schaefer1000x7_TrimMean80', @@ -30,12 +31,16 @@ markers = [ 'method': 'std'} ] +storage = { + 'kind': 'SQLiteFeatureStorage', +} + with tempfile.TemporaryDirectory() as tmpdir: uri = f'{tmpdir}/test.db' + storage['uri'] = uri run( workdir='/tmp', - datagrabber='OasisVBMTestingDatagrabber', + datagrabber=datagrabber, markers=markers, - storage='SQLiteFeatureStorage', - storage_params={'uri': uri}, + storage=storage, ) diff --git a/examples/yamls/gmd_mean.yaml b/examples/yamls/gmd_mean.yaml new file mode 100644 index 000000000..ce50d5d5c --- /dev/null +++ b/examples/yamls/gmd_mean.yaml @@ -0,0 +1,25 @@ +with: junifer.testing.registry +workdir: /tmp + +datagrabber: + kind: OasisVBMTestingDatagrabber +elements: +markers: + - name: Schaefer1000x7_TrimMean80 + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: trim_mean + method_params: + proportiontocut: 0.2 + - name: Schaefer1000x7_Mean + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: mean + - name: Schaefer1000x7_Std + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: std +storage: + kind: SQLiteFeatureStorage + uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db + diff --git a/junifer/api/__init__.py b/junifer/api/__init__.py index 4bc42cc57..9ab1c35b8 100644 --- a/junifer/api/__init__.py +++ b/junifer/api/__init__.py @@ -1 +1,2 @@ -from . run import run \ No newline at end of file +from . run import run +from . import cli \ No newline at end of file diff --git a/junifer/api/cli.py b/junifer/api/cli.py new file mode 100644 index 000000000..fcc3da991 --- /dev/null +++ b/junifer/api/cli.py @@ -0,0 +1,49 @@ +import click + +from .parser import parse_yaml +from .run import run as api_run +from .run import collect as api_collect +from ..utils.logging import configure_logging + + +@click.group() +def cli(): + pass + + +@cli.command() +@click.argument('filepath') +@click.option('-v', '--verbose', + type=click.Choice(['warning', 'info', 'debug'], + case_sensitive=False), + default='warning') +def run(filepath, verbose): + configure_logging(level=verbose.upper()) + contents = parse_yaml(filepath) + workdir = contents['workdir'] + datagrabber = contents['datagrabber'] + markers = contents['markers'] + storage = contents['storage'] + elements = contents.get('elements', None) + api_run( + workdir=workdir, datagrabber=datagrabber, markers=markers, + storage=storage, elements=elements) + + +@cli.command() +@click.argument('filepath') +@click.option('-v', '--verbose', + type=click.Choice(['warning', 'info', 'debug'], + case_sensitive=False), + default='warning') +def collect(filepath, verbose): + configure_logging(level=verbose.upper()) + contents = parse_yaml(filepath) + storage = contents['storage'] + api_collect(storage) + + + +@cli.command() +def queue(): + click.echo('queue') diff --git a/junifer/api/parser.py b/junifer/api/parser.py new file mode 100644 index 000000000..03b041362 --- /dev/null +++ b/junifer/api/parser.py @@ -0,0 +1,23 @@ +import yaml +import importlib +from pathlib import Path + +from ..utils.logging import raise_error + + +def parse_yaml(filepath): + if not isinstance(filepath, Path): + filepath = Path(filepath) + if not filepath.exists(): + raise_error(f'File does not exist: {filepath.as_posix()}') + with open(filepath, 'r') as f: + contents = yaml.safe_load(f) + + if 'with' in contents: + to_load = contents.pop('with') + if not isinstance(to_load, list): + to_load = [to_load] + for t_module in to_load: + importlib.import_module(t_module) + + return contents diff --git a/junifer/api/run.py b/junifer/api/run.py index f33737a33..f26957272 100644 --- a/junifer/api/run.py +++ b/junifer/api/run.py @@ -12,16 +12,17 @@ from ..markers.collection import MarkerCollection def run( - workdir, datagrabber, markers, storage, source_params=None, - storage_params=None, elements=None): + workdir, datagrabber, markers, storage, elements=None): """Run the pipeline on the selected element Parameters ---------- workdir : str or path-like object Directory where the pipeline will be executed - datagrabber : str - Name of the datagrabber to use + datagrabber : dict + Datagrabber to use. Must have a key 'kind' with the kind of + datagrabber to use. All other keys are passed to the datagrabber + init function. elements : str, tuple or list[str or tuple] Element(s) to process. Will be used to index the datagrabber. markers : list of dict @@ -30,24 +31,22 @@ def run( marker. The 'kind' key is used to specify the kind of marker to extract. The rest of the keys are used to pass parameters to the marker calculation. - storage: str - Name of the storage to use. - source_params : dict - Parameters to pass to the datagrabber. - storage_params: dict - Parameters to pass to the storage. + storage : dict + Storage to use. Must have a key 'kind' with the kind of + storage to use. All other keys are passed to the storage + init function. """ - if source_params is None: - source_params = {} - - if storage_params is None: - storage_params = {} + datagrabber_params = datagrabber.copy() + datagrabber_kind = datagrabber_params.pop('kind') + storage_params = storage.copy() + storage_kind = storage_params.pop('kind') if isinstance(workdir, str): workdir = Path(workdir) datagrabber = build( - 'datagrabber', datagrabber, BaseDataGrabber, init_params=source_params) + 'datagrabber', datagrabber_kind, BaseDataGrabber, + init_params=datagrabber_params) built_markers = [] for t_marker in markers: @@ -56,7 +55,8 @@ def run( built_markers.append(t_m) storage = build( - 'storage', storage, BaseFeatureStorage, init_params=storage_params) + 'storage', storage_kind, BaseFeatureStorage, + init_params=storage_params) mc = MarkerCollection(built_markers, storage=storage) @@ -67,3 +67,13 @@ def run( else: for t_element in datagrabber: mc.fit(datagrabber[t_element]) + + +def collect(storage): + storage_params = storage.copy() + storage_kind = storage_params.pop('kind') + + storage = build( + 'storage', storage_kind, BaseFeatureStorage, + init_params=storage_params) + storage.collect() diff --git a/junifer/markers/tests/test_collection.py b/junifer/markers/tests/test_collection.py index 969f356e1..07f601992 100644 --- a/junifer/markers/tests/test_collection.py +++ b/junifer/markers/tests/test_collection.py @@ -7,7 +7,7 @@ from numpy.testing import assert_array_equal from junifer.datareader.default import DefaultDataReader from junifer.markers import MarkerCollection, ParcelAggregation from junifer.markers.base import PipelineStepMixin -from junifer.testing import OasisVBMTestingDatagrabber +from junifer.testing.datagrabbers import OasisVBMTestingDatagrabber from junifer.storage import SQLiteFeatureStorage diff --git a/junifer/storage/base.py b/junifer/storage/base.py index c9a326d6e..f995bd13f 100644 --- a/junifer/storage/base.py +++ b/junifer/storage/base.py @@ -10,7 +10,7 @@ from ..utils import logger from .. import __version__ -def process_meta(meta, return_idx=False, n_rows=1, rows_col_name=None): +def process_meta(meta): """Process the metadata for storage. It removes the "element" key and adds the "_element_keys" with the keys used to index the element. @@ -38,10 +38,6 @@ def process_meta(meta, return_idx=False, n_rows=1, rows_col_name=None): if meta is None: raise ValueError('Meta must be a dict (currently is None)') t_meta = meta.copy() - idx = None - if return_idx is True: - idx = _element_to_index( - meta, n_rows=n_rows, rows_col_name=rows_col_name) element = t_meta.pop('element', None) if element is None: if '_element_keys' not in t_meta: @@ -52,14 +48,8 @@ def process_meta(meta, return_idx=False, n_rows=1, rows_col_name=None): t_meta['_element_keys'] = list(element.keys()) else: t_meta['_element_keys'] = ['element'] - logger.debug(f'Hasing meta {t_meta}') md5_hash = _meta_hash(t_meta) - logger.debug(f'Hash computed: {md5_hash}') - if return_idx is True: - out = md5_hash, t_meta, idx - else: - out = md5_hash, t_meta - return out + return md5_hash, t_meta def _meta_hash(meta): @@ -75,12 +65,15 @@ def _meta_hash(meta): md5: str The md5 hash of the meta """ + logger.debug(f'Hashing meta {meta}') meta_md5 = hashlib.md5( json.dumps(meta, sort_keys=True).encode('utf-8')).hexdigest() + logger.debug(f'Hash computed: {meta_md5}') return meta_md5 -def _element_to_index(meta, n_rows=1, rows_col_name=None): +# TODO: Test this new functionality +def element_to_index(meta, n_rows=1, rows_col_name=None): """Convert the element meta to index Parameters @@ -88,7 +81,7 @@ def _element_to_index(meta, n_rows=1, rows_col_name=None): meta: dict The metadata. Must contain the key 'element' n_rows: int - Number of rows to create + Number of rows to create. Defaults to 1. rows_col_name: str The column name to use in case n_rows > 1. If None (default) and n_rows > 1, the name will be 'index'. @@ -109,32 +102,30 @@ def _element_to_index(meta, n_rows=1, rows_col_name=None): element = meta['element'] if not isinstance(element, dict): element = dict(element=element) - if n_rows > 1: - if rows_col_name is None: - rows_col_name = 'index' - elem_idx = { - k: [v] * n_rows for k, v in element.items() - } - elem_idx[rows_col_name] = np.arange(n_rows) # type: ignore - else: - elem_idx = element + if rows_col_name is None: + rows_col_name = 'idx' + elem_idx = { + k: [v] * n_rows for k, v in element.items() + } + elem_idx[rows_col_name] = np.arange(n_rows) # type: ignore index = pd.MultiIndex.from_frame( pd.DataFrame(elem_idx, index=range(n_rows))) - return index def element_to_prefix(element): + logger.debug(f'Converting element {element} to prefix') prefix = 'element' - if isinstance(element, str): - prefix = f'{prefix}_{element}' - elif isinstance(element, tuple): - prefix = f"{prefix}_{'_'.join(element)}" + if isinstance(element, tuple): + prefix = f"{prefix}_{'_'.join([f'{x}' for x in element])}" elif isinstance(element, dict): - prefix = f"{prefix}_{'_'.join(element.values())}" + prefix = f"{prefix}_{'_'.join([f'{x}' for x in element.values()])}" + elif isinstance(element, (str, int)): + prefix = f'{prefix}_{element}' else: raise ValueError(f'Cannot convert element {element} to prefix. ' 'Must be a str, tuple or dict') + logger.debug(f'Converted prefix {prefix}') return f'{prefix}_' @@ -214,6 +205,10 @@ class BaseFeatureStorage(ABC): def store_timeseries(self, data, meta): raise NotImplementedError('store_timeseries not implemented') + @abstractmethod + def collect(self): + raise NotImplementedError('collect not implemented') + class PandasFeatureStoreage(BaseFeatureStorage): diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index d872d9978..389141d24 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -7,8 +7,9 @@ from pandas.io.sql import pandasSQL_builder from sqlalchemy import create_engine, inspect from ..api.decorators import register_storage -from .base import PandasFeatureStoreage, process_meta, element_to_prefix -from ..utils.logging import warn +from .base import (PandasFeatureStoreage, process_meta, element_to_prefix, + element_to_index) +from ..utils.logging import warn, logger @register_storage @@ -93,7 +94,7 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): raise ValueError('At least one of feature_name or feature_md5 ' 'must be specified') elif feature_md5 is not None: - feature_md5 = f'meta_{feature_md5}' + table_name = f'meta_{feature_md5}' else: meta_df = pd.read_sql( 'meta', con=engine, index_col='meta_md5') @@ -105,14 +106,22 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): f'More than one feature with name {feature_name} found', 'This file is invalid. You can bypass this issue by ' 'specifying a feature_md5') - feature_md5 = f'meta_{t_df.index[0]}' - return pd.read_sql(feature_md5, con=engine) + table_name = f'meta_{t_df.index[0]}' + df = pd.read_sql(table_name, con=engine) + # Read the index: + query = ("SELECT ii.name FROM sqlite_schema AS m, " + "pragma_index_list(m.name) AS il, " + "pragma_index_info(il.name) AS ii " + f"WHERE tbl_name='{table_name}' " + "ORDER BY cid;") + index_names = pd.read_sql(query, con=engine).values.squeeze().tolist() + df = df.set_index(index_names) + return df def store_metadata(self, meta): t_meta = meta.copy() t_meta.update(self.get_meta()) - meta_md5, t_meta_row = process_meta( # type: ignore - t_meta, return_idx=False) + meta_md5, t_meta_row = process_meta(t_meta) engine = self.get_engine(t_meta) if meta_md5 not in inspect(engine).get_table_names(): meta_df = self._meta_row(t_meta_row, meta_md5) @@ -129,22 +138,35 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): def store_2d(self, data, meta, columns=None, rows_col_name=None): n_rows = len(data) - _, _, idx = process_meta( # type: ignore - meta, return_idx=True, n_rows=n_rows, rows_col_name=rows_col_name) + idx = element_to_index( + meta, n_rows=n_rows, rows_col_name=rows_col_name) data_df = pd.DataFrame(data, columns=columns, index=idx) self.store_df(data_df, meta) def store_df(self, df, meta): - table_name = self.store_metadata(meta) - # Check that the index generated by meta is in the dataframe, at least - # for one row - _, _, idx = process_meta( # type: ignore - meta, return_idx=True, n_rows=1) - if any(x not in df.index.names for x in idx.names): # type: ignore + # TODO: Test this function + # Check that the index generated by meta matches the one in + # the dataframe. + idx = element_to_index(meta) + # Given the meta, we might not know if there is an extra column added + # when storing a timeseries or 2d elements. We need to check if the + # extra element is only one. + extra = [x for x in df.index.names if x not in idx.names] + if len(extra) > 1: raise ValueError( - 'The index of the dataframe does not match the ' - 'index of the meta data. This happens when the element ' - 'is not part of the index.') + 'The index of the dataframe has extra items that are not ' + 'in the index generated from the meta data.') + elif len(extra) == 1: + # The df has one extra index item, this should be the new name + # of the missing element in the index + idx = element_to_index(meta, rows_col_name=extra[0]) + + if any(x not in df.index.names for x in idx.names): + raise ValueError( + 'The index of the dataframe is missing index items that are ' + 'generated from the meta data.') + + table_name = self.store_metadata(meta) # Save engine = self.get_engine(meta) self._save_upsert(df, table_name, engine) @@ -152,6 +174,31 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): def store_timeseries(self, data, meta): raise NotImplementedError('store_timeseries not implemented') + def collect(self): + if self.single_output is True: + raise ValueError('collect is not implemented for single output') + logger.info( + f'Collecting data from {self.uri.parent}/*{self.uri.name}') + + out_storage = SQLiteFeatureStorage( + uri=self.uri, single_output=True, upsert='ignore') + + for elem in self.uri.parent.glob(f'*{self.uri.name}'): + logger.debug(f'Reading from {elem.as_posix()}') + in_storage = SQLiteFeatureStorage(uri=elem, single_output=True) + in_engine = in_storage.get_engine() + # Open "meta" table + t_meta_df = pd.read_sql( + 'meta', con=in_engine, index_col='meta_md5') + out_storage._save_upsert(t_meta_df, 'meta') + for meta_md5 in t_meta_df.index: + logger.debug(f'Collecting feature {meta_md5}') + # TODO: Fix this, needs that read_feature sets the index + # properly + table_name = f'meta_{meta_md5}' + t_df = in_storage.read_df(feature_md5=meta_md5) + out_storage._save_upsert(t_df, table_name) + def _save_upsert(self, df, name, engine=None, if_exist='append'): """ Implementation of UPSERT functionality. @@ -217,7 +264,7 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): def _get_existing_pk(con, table_name, index_col): - pk_cols = ','.join(index_col) + pk_cols = ', '.join(index_col) query = f'SELECT {pk_cols} FROM {table_name};' pk_indb = pd.read_sql(query, con=con) return pk_indb diff --git a/junifer/storage/tests/test_base.py b/junifer/storage/tests/test_base.py index 01f2156f8..532907250 100644 --- a/junifer/storage/tests/test_base.py +++ b/junifer/storage/tests/test_base.py @@ -1,7 +1,8 @@ import pytest -from junifer.storage.base import (process_meta, _element_to_index, - BaseFeatureStorage) +from junifer.storage.base import (process_meta, element_to_index, + BaseFeatureStorage, + element_to_prefix) def test_process_meta_hash(): @@ -12,14 +13,14 @@ def test_process_meta_hash(): process_meta(meta) meta = {'element': 'foo', 'A': 1, 'B': [2, 3, 4, 5, 6]} - hash1, _ = process_meta(meta, return_idx=False) # type: ignore + hash1, _ = process_meta(meta) meta = {'element': 'foo', 'B': [2, 3, 4, 5, 6], 'A': 1} - hash2, _ = process_meta(meta, return_idx=False) # type: ignore + hash2, _ = process_meta(meta) assert hash1 == hash2 meta = {'element': 'foo', 'A': 1, 'B': [2, 3, 1, 5, 6]} - hash3, _ = process_meta(meta, return_idx=False) # type: ignore + hash3, _ = process_meta(meta) assert hash1 != hash3 meta1 = { @@ -41,8 +42,8 @@ def test_process_meta_hash(): 'element': 'foo' } - hash4, _ = process_meta(meta1, return_idx=False) # type: ignore - hash5, _ = process_meta(meta2, return_idx=False) # type: ignore + hash4, _ = process_meta(meta1) + hash5, _ = process_meta(meta2) assert hash4 == hash5 @@ -51,10 +52,10 @@ def test_process_meta_element(): meta = {} with pytest.raises(ValueError, match=r"_element_keys"): - process_meta(meta, return_idx=False) + process_meta(meta) meta = {'element': 'foo', 'A': 1, 'B': [2, 3, 4, 5, 6]} - _, new_meta = process_meta(meta, return_idx=False) # type: ignore + _, new_meta = process_meta(meta) assert '_element_keys' in new_meta assert new_meta['_element_keys'] == ['element'] assert 'A' in new_meta @@ -63,7 +64,7 @@ def test_process_meta_element(): meta = { 'element': {'subject': 'foo', 'session': 'bar'}, 'B': [2, 3, 4, 5, 6], 'A': 1} - _, new_meta = process_meta(meta, return_idx=False) # type: ignore + _, new_meta = process_meta(meta) assert '_element_keys' in new_meta assert new_meta['_element_keys'] == ['subject', 'session'] assert 'A' in new_meta @@ -71,35 +72,44 @@ def test_process_meta_element(): def test_process_meta_index(): - """Test _element_to_index""" + """Test element_to_index""" meta = {'noelement': 'foo'} with pytest.raises(ValueError, match=r'meta must contain the key'): - _element_to_index(meta) + element_to_index(meta) meta = {'element': 'foo', 'A': 1, 'B': [2, 3, 4, 5, 6]} - index = _element_to_index(meta) - assert index.names == ['element'] + index = element_to_index(meta) + assert index.names == ['element', 'idx'] assert index.levels[0].name == 'element' assert index.levels[0].values[0] == 'foo' + assert all(x == 'foo' for x in index.levels[0].values) + assert index.levels[0].values.shape == (1,) + assert index.levels[1].name == 'idx' + assert all(x == i for i, x in enumerate(index.levels[1].values)) + assert index.levels[1].values.shape == (1,) - index = _element_to_index(meta, n_rows=10) - assert index.names == ['element', 'index'] + index = element_to_index(meta, n_rows=10) + assert index.names == ['element', 'idx'] assert index.levels[0].name == 'element' assert all(x == 'foo' for x in index.levels[0].values) assert index.levels[0].values.shape == (1,) - assert index.levels[1].name == 'index' + assert index.levels[1].name == 'idx' assert all(x == i for i, x in enumerate(index.levels[1].values)) assert index.levels[1].values.shape == (10,) - index = _element_to_index(meta, n_rows=1, rows_col_name='scan') - assert index.names == ['element'] + index = element_to_index(meta, n_rows=1, rows_col_name='scan') + assert index.names == ['element', 'scan'] assert index.levels[0].name == 'element' + assert index.levels[0].values[0] == 'foo' assert all(x == 'foo' for x in index.levels[0].values) assert index.levels[0].values.shape == (1,) + assert index.levels[1].name == 'scan' + assert all(x == i for i, x in enumerate(index.levels[1].values)) + assert index.levels[1].values.shape == (1,) - index = _element_to_index(meta, n_rows=7, rows_col_name='scan') + index = element_to_index(meta, n_rows=7, rows_col_name='scan') assert index.names == ['element', 'scan'] assert index.levels[0].name == 'element' assert all(x == 'foo' for x in index.levels[0].values) @@ -112,7 +122,7 @@ def test_process_meta_index(): meta = { 'element': {'subject': 'sub-01', 'session': 'ses-01'}, 'A': 1, 'B': [2, 3, 4, 5, 6]} - index = _element_to_index(meta, n_rows=10) + index = element_to_index(meta, n_rows=10) assert index.levels[0].name == 'subject' assert all(x == 'sub-01' for x in index.levels[0].values) @@ -122,7 +132,7 @@ def test_process_meta_index(): assert all(x == 'ses-01' for x in index.levels[1].values) assert index.levels[1].values.shape == (1,) - assert index.levels[2].name == 'index' + assert index.levels[2].name == 'idx' assert all(x == i for i, x in enumerate(index.levels[2].values)) assert index.levels[2].values.shape == (10,) @@ -161,6 +171,9 @@ def test_BaseFeatureStorage(): def store_timeseries(self, timeseries, meta): super().store_timeseries(timeseries, meta) + def collect(self): + return super().collect() + st = MyFeatureStorage(uri='/tmp') assert st.single_output is False @@ -191,4 +204,43 @@ def test_BaseFeatureStorage(): with pytest.raises(NotImplementedError): st.store_timeseries(None, None) + with pytest.raises(NotImplementedError): + st.collect() + assert st.uri == '/tmp' + + +def test_element_to_prefix(): + """Test converting element to prefix (for file naming)""" + + element = 'sub-01' + prefix = element_to_prefix(element) + assert prefix == 'element_sub-01_' + + element = 1 + prefix = element_to_prefix(element) + assert prefix == 'element_1_' + + element = {'subject': 'sub-01'} + prefix = element_to_prefix(element) + assert prefix == 'element_sub-01_' + + element = {'subject': 1} + prefix = element_to_prefix(element) + assert prefix == 'element_1_' + + element = {'subject': 'sub-01', 'session': 'ses-02'} + prefix = element_to_prefix(element) + assert prefix == 'element_sub-01_ses-02_' + + element = {'subject': 1, 'session': 2} + prefix = element_to_prefix(element) + assert prefix == 'element_1_2_' + + element = ('sub-01', 'ses-02') + prefix = element_to_prefix(element) + assert prefix == 'element_sub-01_ses-02_' + + element = (1, 2) + prefix = element_to_prefix(element) + assert prefix == 'element_1_2_' diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index 31abd542d..6d82c5993 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -7,7 +7,8 @@ from sqlalchemy import create_engine import pytest from junifer.storage.sqlite import SQLiteFeatureStorage -from junifer.storage.base import process_meta, element_to_prefix +from junifer.storage.base import (process_meta, element_to_prefix, + element_to_index) df1 = pd.DataFrame({ @@ -160,11 +161,10 @@ def test_store_read_df(): to_store = df1[['col1', 'col2']] # Save to SQL - with pytest.raises(ValueError, match=r"index of the dataframe"): + with pytest.raises(ValueError, match=r"missing index items"): storage.store_df(to_store.set_index('col1'), meta) - _, _, idx = process_meta( # type: ignore - meta, return_idx=True, n_rows=len(to_store)) + idx = element_to_index(meta, n_rows=len(to_store)) to_store = to_store.set_index(idx) storage.store_df(to_store, meta) @@ -190,7 +190,7 @@ def test_store_read_df(): read_df1 = storage.read_df(feature_md5=feature_md5) read_df2 = storage.read_df(feature_name='fcname') assert_frame_equal(read_df1, read_df2) - assert_frame_equal(read_df1, to_store.reset_index()) + assert_frame_equal(read_df1, to_store) def test_store_table(): @@ -206,8 +206,8 @@ def test_store_table(): [4, 40], [5, 50], ] - _, _, idx = process_meta( # type: ignore - meta, return_idx=True, n_rows=5, rows_col_name='scan') + + idx = element_to_index(meta, n_rows=5, rows_col_name='scan') df1 = pd.DataFrame(data, columns=['f1', 'f2'], index=idx) storage.store_table( @@ -226,8 +226,7 @@ def test_store_table(): [6, 600] ] - _, _, idx = process_meta( # type: ignore - meta, return_idx=True, n_rows=6, rows_col_name='scan') + idx = element_to_index(meta, n_rows=6, rows_col_name='scan') df2 = pd.DataFrame(data2, columns=['f1', 'f2'], index=idx) with pytest.warns(RuntimeWarning, match=r"Some rows"): @@ -261,16 +260,16 @@ def test_store_multiple_output(): data2 = data1 * 10 data3 = data1 * 20 - hash1, _, idx1 = process_meta( # type: ignore - meta1, return_idx=True, n_rows=5, rows_col_name='scan') + hash1, _ = process_meta(meta1) + idx1 = element_to_index(meta1, n_rows=5, rows_col_name='scan') df1 = pd.DataFrame(data1, columns=['f1', 'f2'], index=idx1) - hash2, _, idx2 = process_meta( # type: ignore - meta2, return_idx=True, n_rows=5, rows_col_name='scan') + hash2, _ = process_meta(meta2) + idx2 = element_to_index(meta2, n_rows=5, rows_col_name='scan') df2 = pd.DataFrame(data2, columns=['f1', 'f2'], index=idx2) - hash3, _, idx3 = process_meta( # type: ignore - meta3, return_idx=True, n_rows=5, rows_col_name='scan') + hash3, _ = process_meta(meta3) + idx3 = element_to_index(meta3, n_rows=5, rows_col_name='scan') df3 = pd.DataFrame(data3, columns=['f1', 'f2'], index=idx3) assert hash1 == hash2 diff --git a/junifer/testing/__init__.py b/junifer/testing/__init__.py new file mode 100644 index 000000000..4b2ba56f9 --- /dev/null +++ b/junifer/testing/__init__.py @@ -0,0 +1,3 @@ +# Authors: Federico Raimondo +# License: AGPL +from . import datagrabbers \ No newline at end of file diff --git a/junifer/testing/datagrabbers.py b/junifer/testing/datagrabbers.py new file mode 100644 index 000000000..fba285064 --- /dev/null +++ b/junifer/testing/datagrabbers.py @@ -0,0 +1,29 @@ +# Authors: Federico Raimondo +# License: AGPL +import tempfile +from nilearn import datasets + +from ..datagrabber.base import BaseDataGrabber + + +class OasisVBMTestingDatagrabber(BaseDataGrabber): + """ + DataGrabber for Oasis VBM testing data. + """ + def __init__(self): + datadir = tempfile.mkdtemp() + types = ['VBM_GM'] + super().__init__(types=types, datadir=datadir) + + def get_elements(self): + return list(range(1, 11)) + + def __getitem__(self, element): + out = {} + out['VBM_GM'] = self._dataset.gray_matter_maps[element - 1] + out['meta'] = {'element': {'subject': element}} + return out + + def __enter__(self): + self._dataset = datasets.fetch_oasis_vbm(n_subjects=10) + return self diff --git a/junifer/testing/registry.py b/junifer/testing/registry.py new file mode 100644 index 000000000..556005b53 --- /dev/null +++ b/junifer/testing/registry.py @@ -0,0 +1,6 @@ +from .datagrabbers import OasisVBMTestingDatagrabber +from ..api.registry import register + +register( + 'datagrabber', 'OasisVBMTestingDatagrabber', + OasisVBMTestingDatagrabber) diff --git a/requirements.txt b/requirements.txt index eca50d3f0..2eb1a7062 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,5 @@ datalad>=0.15.4, <0.16 pandas>=0.18.0, <1.5 nibabel>=3.2.0, <4.0 nilearn>=0.9.0, <1.0 -sqlalchemy>=1.4.27, <= 1.5.0 \ No newline at end of file +sqlalchemy>=1.4.27, <= 1.5.0 +pyyaml>=5.1.2, <7.0 \ No newline at end of file diff --git a/setup.py b/setup.py index 846a52937..1c3f673e2 100644 --- a/setup.py +++ b/setup.py @@ -52,7 +52,13 @@ setuptools.setup( 'Source': DOWNLOAD_URL, 'Tracker': f'{DOWNLOAD_URL}issues/', }, - install_requires=[], # TODO: Complete + install_requires=['Click'], # TODO: Complete + py_modules=['junifer.api.cli'], + entry_points={ + 'console_scripts': [ + 'junifer=junifer.api.cli:cli', + ] + }, python_requires='>=3.6', use_scm_version=_getversion, setup_requires=['setuptools_scm'], -- 2.52.0 From 4e1d118dd61613b2ffd22245f44cd172a493b344 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 11:48:57 +0100 Subject: [PATCH 04/40] Flake --- junifer/api/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/junifer/api/cli.py b/junifer/api/cli.py index fcc3da991..6a14bfd2a 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -43,7 +43,6 @@ def collect(filepath, verbose): api_collect(storage) - @cli.command() def queue(): click.echo('queue') -- 2.52.0 From dd0077f7515e4313d4a03fe8d98253fcf51925b7 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 12:02:34 +0100 Subject: [PATCH 05/40] Update reqs + use sqlite_master --- dev-requirements.txt | 3 ++- junifer/storage/sqlite.py | 2 +- junifer/storage/tests/test_sqlite.py | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index aad120b7c..2e4e347ff 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,2 +1,3 @@ flake8 -pytest \ No newline at end of file +pytest +click \ No newline at end of file diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index 389141d24..9616ff7f6 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -109,7 +109,7 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): table_name = f'meta_{t_df.index[0]}' df = pd.read_sql(table_name, con=engine) # Read the index: - query = ("SELECT ii.name FROM sqlite_schema AS m, " + query = ("SELECT ii.name FROM sqlite_master AS m, " "pragma_index_list(m.name) AS il, " "pragma_index_info(il.name) AS ii " f"WHERE tbl_name='{table_name}' " diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index 6d82c5993..de185954f 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -115,7 +115,8 @@ def test_upsert_ignore(): c_df1 = _read_sql(table_name, uri=uri, index_col=['element', 'pk2']) assert_frame_equal(df1, c_df1) - storage.store_df(df2, meta) + with pytest.warns(RuntimeWarning, match='are already present'): + storage.store_df(df2, meta) c_dfignore = _read_sql( table_name, uri=uri, index_col=['element', 'pk2']) -- 2.52.0 From 5e1dfdbff45d4e5c27291d3347b22a42006948e9 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 12:19:15 +0100 Subject: [PATCH 06/40] Test collect --- .coveragerc | 2 +- junifer/storage/tests/test_sqlite.py | 72 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/.coveragerc b/.coveragerc index 6cdbc7a3c..2cd7255d0 100644 --- a/.coveragerc +++ b/.coveragerc @@ -6,7 +6,7 @@ omit = */setup.py */tests/* junifer/configs/juseless.py - junifer/testing.py + junifer/testing/* [report] exclude_lines = diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index de185954f..cde4e35ea 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -58,6 +58,11 @@ def test_get_engine(): assert engine.url.drivername == 'sqlite' assert f'{engine.url.database}' == uri + storage = SQLiteFeatureStorage( + uri=uri, single_output=False, upsert='ignore') + with pytest.raises(ValueError, match='element must be specified'): + storage.get_engine() + def test_store_metadata(): """Test store_metadata""" @@ -165,6 +170,10 @@ def test_store_read_df(): with pytest.raises(ValueError, match=r"missing index items"): storage.store_df(to_store.set_index('col1'), meta) + to_store = df1.reset_index.set_index(['element', 'pk2', 'col1']) + with pytest.raises(ValueError, match=r"extra items"): + storage.store_df(to_store.set_index('col1'), meta) + idx = element_to_index(meta, n_rows=len(to_store)) to_store = to_store.set_index(idx) @@ -310,3 +319,66 @@ def test_store_multiple_output(): assert_frame_equal(df1, cdf1) assert_frame_equal(df2, cdf2) assert_frame_equal(df3, cdf3) + + +def test_collect(): + meta1 = {'element': {'subject': 'test-01', 'session': 'ses-01'}, + 'version': '0.0.1', 'marker': {'name': 'fc'}} + meta2 = {'element': {'subject': 'test-02', 'session': 'ses-01'}, + 'version': '0.0.1', 'marker': {'name': 'fc'}} + meta3 = {'element': {'subject': 'test-01', 'session': 'ses-02'}, + 'version': '0.0.1', 'marker': {'name': 'fc'}} + with tempfile.TemporaryDirectory() as _tmpdir: + uri = Path(f'{_tmpdir}/test.db') + storage = SQLiteFeatureStorage(uri=uri) + + data1 = np.array([ + [1, 10], + [2, 20], + [3, 30], + [4, 40], + [5, 50], + ]) + + data2 = data1 * 10 + data3 = data1 * 20 + storage.store_table( + data1, meta1, columns=['f1', 'f2'], rows_col_name='scan') + + storage.store_table( + data2, meta2, columns=['f1', 'f2'], rows_col_name='scan') + + storage.store_table( + data3, meta3, columns=['f1', 'f2'], rows_col_name='scan') + + prefix1 = element_to_prefix(meta1['element']) + prefix2 = element_to_prefix(meta2['element']) + prefix3 = element_to_prefix(meta3['element']) + + uri1 = uri.parent / f'{prefix1}{uri.name}' + uri2 = uri.parent / f'{prefix2}{uri.name}' + uri3 = uri.parent / f'{prefix3}{uri.name}' + + assert uri1.exists() + assert uri2.exists() + assert uri3.exists() + + assert not uri.exists() + + storage.collect() + + assert uri.exists() + + cols = ['subject', 'session', 'scan'] + table_name = storage.store_metadata(meta1) + all_df = _read_sql(table_name, uri, index_col=cols) + + cdf1 = _read_sql(table_name, uri1, index_col=cols) + cdf2 = _read_sql(table_name, uri2, index_col=cols) + cdf3 = _read_sql(table_name, uri3, index_col=cols) + + all_cdf = pd.concat([cdf1, cdf2, cdf3]) + all_df.sort_index(level=cols, inplace=True) + all_cdf.sort_index(level=cols, inplace=True) + + assert_frame_equal(all_df, all_cdf) -- 2.52.0 From a39cff7d1cd9d1505926adae0874734bf8863aa6 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 12:24:04 +0100 Subject: [PATCH 07/40] typo --- junifer/storage/tests/test_sqlite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index cde4e35ea..558cff610 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -170,7 +170,7 @@ def test_store_read_df(): with pytest.raises(ValueError, match=r"missing index items"): storage.store_df(to_store.set_index('col1'), meta) - to_store = df1.reset_index.set_index(['element', 'pk2', 'col1']) + to_store = df1.reset_index().set_index(['element', 'pk2', 'col1']) with pytest.raises(ValueError, match=r"extra items"): storage.store_df(to_store.set_index('col1'), meta) -- 2.52.0 From 56382b543b3a354b5364d2da3a62c4d4d35e7e5b Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 12:44:29 +0100 Subject: [PATCH 08/40] getting tired of testing sqlite --- junifer/storage/tests/test_sqlite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index 558cff610..55e3670e7 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -172,7 +172,7 @@ def test_store_read_df(): to_store = df1.reset_index().set_index(['element', 'pk2', 'col1']) with pytest.raises(ValueError, match=r"extra items"): - storage.store_df(to_store.set_index('col1'), meta) + storage.store_df(to_store, meta) idx = element_to_index(meta, n_rows=len(to_store)) to_store = to_store.set_index(idx) -- 2.52.0 From bdf4c764a131533c894614a5cb94bb2d0868ab71 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 14:18:04 +0100 Subject: [PATCH 09/40] More tests --- junifer/api/__init__.py | 4 +- junifer/api/cli.py | 4 +- junifer/api/{run.py => functions.py} | 0 junifer/api/registry.py | 6 +-- junifer/api/tests/test_functions.py | 0 junifer/api/tests/test_parser.py | 39 +++++++++++++++++++ junifer/api/tests/test_registry.py | 56 ++++++++++++++++++++++++++++ 7 files changed, 101 insertions(+), 8 deletions(-) rename junifer/api/{run.py => functions.py} (100%) create mode 100644 junifer/api/tests/test_functions.py create mode 100644 junifer/api/tests/test_parser.py create mode 100644 junifer/api/tests/test_registry.py diff --git a/junifer/api/__init__.py b/junifer/api/__init__.py index 9ab1c35b8..ecd3d2c69 100644 --- a/junifer/api/__init__.py +++ b/junifer/api/__init__.py @@ -1,2 +1,2 @@ -from . run import run -from . import cli \ No newline at end of file +from .functions import run, collect +from .import cli \ No newline at end of file diff --git a/junifer/api/cli.py b/junifer/api/cli.py index 6a14bfd2a..c32b21484 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -1,8 +1,8 @@ import click from .parser import parse_yaml -from .run import run as api_run -from .run import collect as api_collect +from .functions import run as api_run +from .functions import collect as api_collect from ..utils.logging import configure_logging diff --git a/junifer/api/run.py b/junifer/api/functions.py similarity index 100% rename from junifer/api/run.py rename to junifer/api/functions.py diff --git a/junifer/api/registry.py b/junifer/api/registry.py index 3de096073..922433f7c 100644 --- a/junifer/api/registry.py +++ b/junifer/api/registry.py @@ -93,14 +93,12 @@ def build(step, name, baseclass, init_params=None): If the name is not a string or the object is not an instance of the baseclass parameter. """ - if not isinstance(name, str): - raise_error(f'Invalid name: {name}', ValueError) klass = get(step, name) if init_params is None: init_params = {} object = klass(**init_params) if not isinstance(object, baseclass): raise_error( - f'Invalid {step} ({object.__class__.name}). ' - f'Must inherit from {baseclass.name}', ValueError) + f'Invalid {step} ({object.__class__.__name__}). ' + f'Must inherit from {baseclass.__name__}', ValueError) return object diff --git a/junifer/api/tests/test_functions.py b/junifer/api/tests/test_functions.py new file mode 100644 index 000000000..e69de29bb diff --git a/junifer/api/tests/test_parser.py b/junifer/api/tests/test_parser.py new file mode 100644 index 000000000..32447506f --- /dev/null +++ b/junifer/api/tests/test_parser.py @@ -0,0 +1,39 @@ +import sys +from pathlib import Path +import tempfile +import pytest + +from junifer.api.parser import parse_yaml + + +def test_parse_yaml(): + """Test parse yaml""" + with pytest.raises(ValueError, match='does not exist'): + parse_yaml('foo.yaml') + + with tempfile.TemporaryDirectory() as _tmpdir: + fname = Path(_tmpdir) / 'test.yaml' + with open(fname, 'w') as f: + f.write('foo: bar\n') + f.write('with: junifer.configs.juseless\n') + + assert 'junifer.configs.juseless' not in sys.modules + contents = parse_yaml(fname) + assert 'foo' in contents + assert 'bar' == contents['foo'] + assert 'with' not in contents + assert 'junifer.configs.juseless' in sys.modules + + assert 'junifer.testing.registry' not in sys.modules + + with open(fname, 'w') as f: + f.write('foo: bar\n') + f.write('with:\n') + f.write(' - junifer.configs.juseless\n') + f.write(' - junifer.testing.registry\n') + + contents = parse_yaml(fname.as_posix()) + assert 'foo' in contents + assert 'bar' == contents['foo'] + assert 'with' not in contents + assert 'junifer.testing.registry' in sys.modules diff --git a/junifer/api/tests/test_registry.py b/junifer/api/tests/test_registry.py new file mode 100644 index 000000000..d8716f541 --- /dev/null +++ b/junifer/api/tests/test_registry.py @@ -0,0 +1,56 @@ +import pytest +from abc import ABC + +from junifer.api.registry import register, get_step_names, get, build + + +def test_register_error(): + """Test register error""" + with pytest.raises(ValueError, match='Invalid ste'): + register('foo', 'bar', 'baz') + + +def test_gets(): + """Test get""" + with pytest.raises(ValueError, match='Invalid ste'): + get_step_names('foo') + + datagrabbers = get_step_names('datagrabber') + assert 'bar' not in datagrabbers + register('datagrabber', 'bar', 'baz') + datagrabbers = get_step_names('datagrabber') + assert 'bar' in datagrabbers + + with pytest.raises(ValueError, match='Invalid ste'): + get('foo', 'bar') + + with pytest.raises(ValueError, match='Invalid name'): + get('datagrabber', 'foo') + + obj = get('datagrabber', 'bar') + assert obj == 'baz' + + +def test_build(): + """Test building objects from names""" + import numpy as np + + class SuperClass(ABC): + pass + + class ConcreteClass(SuperClass): + def __init__(self, value=1): + self.value = value + + register('datagrabber', 'concrete', ConcreteClass) + + obj = build('datagrabber', 'concrete', SuperClass) + assert isinstance(obj, ConcreteClass) + assert obj.value == 1 + + obj = build('datagrabber', 'concrete', SuperClass, {'value': 2}) + assert isinstance(obj, ConcreteClass) + assert obj.value == 2 + + with pytest.raises(ValueError, match='Must inherit'): + build('datagrabber', 'concrete', np.ndarray) -- 2.52.0 From 5928bc910bcf14c178121373640772e962af0721 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 23 Mar 2022 14:58:09 +0100 Subject: [PATCH 10/40] Test run and collect --- junifer/api/functions.py | 5 +- junifer/api/tests/test_functions.py | 91 +++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/junifer/api/functions.py b/junifer/api/functions.py index f26957272..b98273051 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -47,9 +47,10 @@ def run( datagrabber = build( 'datagrabber', datagrabber_kind, BaseDataGrabber, init_params=datagrabber_params) - + # Copy to avoid changing the original dict + _markers = [x.copy() for x in markers] built_markers = [] - for t_marker in markers: + for t_marker in _markers: kind = t_marker.pop('kind') t_m = build('marker', kind, BaseMarker, init_params=t_marker) built_markers.append(t_m) diff --git a/junifer/api/tests/test_functions.py b/junifer/api/tests/test_functions.py index e69de29bb..5fc65510d 100644 --- a/junifer/api/tests/test_functions.py +++ b/junifer/api/tests/test_functions.py @@ -0,0 +1,91 @@ +import tempfile +from pathlib import Path + +from junifer.api.registry import build +from junifer.api.functions import run, collect +from junifer.datagrabber.base import BaseDataGrabber +import junifer.testing.registry # noqa: F401 + +datagrabber = { + 'kind': 'OasisVBMTestingDatagrabber', +} + +markers = [ + {'name': 'Schaefer1000x7_Mean', + 'kind': 'ParcelAggregation', + 'atlas': 'Schaefer1000x7', + 'method': 'mean'}, + {'name': 'Schaefer1000x7_Std', + 'kind': 'ParcelAggregation', + 'atlas': 'Schaefer1000x7', + 'method': 'std'} +] + +storage = { + 'kind': 'SQLiteFeatureStorage', +} + + +def test_run(): + """Test run function""" + with tempfile.TemporaryDirectory() as tmpdir: + tmp_path = Path(tmpdir) + workdir = tmp_path / 'workdir' + workdir.mkdir() + outdir = tmp_path / 'out' + outdir.mkdir() + uri = outdir / 'test.db' + storage['uri'] = uri # type: ignore + run( + workdir=workdir, + datagrabber=datagrabber, + markers=markers, + storage=storage, + elements=[1] + ) + + files = list(outdir.glob('*.db')) + assert len(files) == 1 + + run( + workdir=workdir.as_posix(), + datagrabber=datagrabber, + markers=markers, + storage=storage, + elements=[1, 3] + ) + + files = list(outdir.glob('*.db')) + assert len(files) == 2 + + +def test_collect(): + """Test run and collect functions""" + + with tempfile.TemporaryDirectory() as tmpdir: + tmp_path = Path(tmpdir) + workdir = tmp_path / 'workdir' + workdir.mkdir() + outdir = tmp_path / 'out' + outdir.mkdir() + uri = outdir / 'test.db' + storage['uri'] = uri # type: ignore + run( + workdir=workdir, + datagrabber=datagrabber, + markers=markers, + storage=storage, + ) + dg = build('datagrabber', datagrabber['kind'], BaseDataGrabber) + elements = dg.get_elements() + + # This should create 10 files + files = list(outdir.glob('*.db')) + assert len(files) == len(elements) + + # But the test.db file should not exist + assert not uri.exists() + collect(storage) + + # Now the file exists + assert uri.exists() -- 2.52.0 From db43d379943ad4c2750bead8798ec864044f3b1b Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 24 Mar 2022 12:56:37 +0100 Subject: [PATCH 11/40] Fix parsing test --- junifer/api/run.py | 79 ++++++++++++++++++++++++++++ junifer/api/tests/data/gmd_mean.yaml | 15 ++++++ junifer/api/tests/test_parser.py | 17 +++--- junifer/storage/base.py | 1 - 4 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 junifer/api/run.py create mode 100644 junifer/api/tests/data/gmd_mean.yaml diff --git a/junifer/api/run.py b/junifer/api/run.py new file mode 100644 index 000000000..f26957272 --- /dev/null +++ b/junifer/api/run.py @@ -0,0 +1,79 @@ +# Authors: Federico Raimondo +# Leonard Sasse +# License: AGPL + +from pathlib import Path + +from .registry import build +from ..datagrabber.base import BaseDataGrabber +from ..markers.base import BaseMarker +from ..storage.base import BaseFeatureStorage +from ..markers.collection import MarkerCollection + + +def run( + workdir, datagrabber, markers, storage, elements=None): + """Run the pipeline on the selected element + + Parameters + ---------- + workdir : str or path-like object + Directory where the pipeline will be executed + datagrabber : dict + Datagrabber to use. Must have a key 'kind' with the kind of + datagrabber to use. All other keys are passed to the datagrabber + init function. + elements : str, tuple or list[str or tuple] + Element(s) to process. Will be used to index the datagrabber. + markers : list of dict + List of markers to extract. Each marker is a dict with at least two + keys: 'name' and 'kind'. The 'name' key is used to name the output + marker. The 'kind' key is used to specify the kind of marker to + extract. The rest of the keys are used to pass parameters to the + marker calculation. + storage : dict + Storage to use. Must have a key 'kind' with the kind of + storage to use. All other keys are passed to the storage + init function. + """ + datagrabber_params = datagrabber.copy() + datagrabber_kind = datagrabber_params.pop('kind') + storage_params = storage.copy() + storage_kind = storage_params.pop('kind') + + if isinstance(workdir, str): + workdir = Path(workdir) + + datagrabber = build( + 'datagrabber', datagrabber_kind, BaseDataGrabber, + init_params=datagrabber_params) + + built_markers = [] + for t_marker in markers: + kind = t_marker.pop('kind') + t_m = build('marker', kind, BaseMarker, init_params=t_marker) + built_markers.append(t_m) + + storage = build( + 'storage', storage_kind, BaseFeatureStorage, + init_params=storage_params) + + mc = MarkerCollection(built_markers, storage=storage) + + with datagrabber: + if elements is not None: + for t_element in elements: + mc.fit(datagrabber[t_element]) + else: + for t_element in datagrabber: + mc.fit(datagrabber[t_element]) + + +def collect(storage): + storage_params = storage.copy() + storage_kind = storage_params.pop('kind') + + storage = build( + 'storage', storage_kind, BaseFeatureStorage, + init_params=storage_params) + storage.collect() diff --git a/junifer/api/tests/data/gmd_mean.yaml b/junifer/api/tests/data/gmd_mean.yaml new file mode 100644 index 000000000..af4df1798 --- /dev/null +++ b/junifer/api/tests/data/gmd_mean.yaml @@ -0,0 +1,15 @@ +with: junifer.testing.registry +workdir: /tmp + +datagrabber: + kind: OasisVBMTestingDatagrabber +elements: +markers: + - name: Schaefer1000x7_Mean + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: mean +storage: + kind: SQLiteFeatureStorage + uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db + diff --git a/junifer/api/tests/test_parser.py b/junifer/api/tests/test_parser.py index 32447506f..a91604d03 100644 --- a/junifer/api/tests/test_parser.py +++ b/junifer/api/tests/test_parser.py @@ -15,25 +15,20 @@ def test_parse_yaml(): fname = Path(_tmpdir) / 'test.yaml' with open(fname, 'w') as f: f.write('foo: bar\n') - f.write('with: junifer.configs.juseless\n') + f.write('with: numpy\n') - assert 'junifer.configs.juseless' not in sys.modules contents = parse_yaml(fname) assert 'foo' in contents assert 'bar' == contents['foo'] assert 'with' not in contents - assert 'junifer.configs.juseless' in sys.modules - assert 'junifer.testing.registry' not in sys.modules + assert 'junifer.configs.wrong_config' not in sys.modules with open(fname, 'w') as f: f.write('foo: bar\n') f.write('with:\n') - f.write(' - junifer.configs.juseless\n') - f.write(' - junifer.testing.registry\n') + f.write(' - numpy\n') + f.write(' - junifer.testing.wrong_config\n') - contents = parse_yaml(fname.as_posix()) - assert 'foo' in contents - assert 'bar' == contents['foo'] - assert 'with' not in contents - assert 'junifer.testing.registry' in sys.modules + with pytest.raises(ImportError, match='wrong_config'): + contents = parse_yaml(fname.as_posix()) diff --git a/junifer/storage/base.py b/junifer/storage/base.py index f995bd13f..da1f26aa5 100644 --- a/junifer/storage/base.py +++ b/junifer/storage/base.py @@ -72,7 +72,6 @@ def _meta_hash(meta): return meta_md5 -# TODO: Test this new functionality def element_to_index(meta, n_rows=1, rows_col_name=None): """Convert the element meta to index -- 2.52.0 From 4b6c0d93f23fb6297d0531b2541c2d51cd8ec7a5 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 24 Mar 2022 14:44:23 +0100 Subject: [PATCH 12/40] Test cli --- Makefile | 2 +- junifer/api/tests/test_cli.py | 38 ++++++++++++++++++++++++++++++++ junifer/api/tests/test_parser.py | 7 ++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 junifer/api/tests/test_cli.py diff --git a/Makefile b/Makefile index f34d0536d..d1b4e3a6c 100644 --- a/Makefile +++ b/Makefile @@ -12,4 +12,4 @@ spellcheck: codespell junifer/ docs/ examples/ test: - pytest -v \ No newline at end of file + pytest -vv --cov=junifer --cov-report html --cov-report term \ No newline at end of file diff --git a/junifer/api/tests/test_cli.py b/junifer/api/tests/test_cli.py new file mode 100644 index 000000000..486393b0c --- /dev/null +++ b/junifer/api/tests/test_cli.py @@ -0,0 +1,38 @@ +from pathlib import Path +import tempfile +import yaml +from junifer.api.cli import run, collect + +from click.testing import CliRunner + + +runner = CliRunner() + + +def _modify_path(tmpdir, in_file): + """Modify the path to use the temporary directory""" + if not isinstance(tmpdir, Path): + tmpdir = Path(tmpdir) + with open(in_file, 'r') as f: + contents = yaml.safe_load(f) + outfile = tmpdir / 'in.yaml' + outdir = tmpdir / 'out' + workdir = tmpdir / 'work' + contents['storage']['uri'] = outdir.as_posix() + contents['workdir'] = workdir.as_posix() + with open(outfile, 'w') as f: + yaml.dump(contents, f) + return outfile + + +def test_run_collect(): + """Test run and collect""" + infile = Path(__file__).parent / 'data' / 'gmd_mean.yaml' + with tempfile.TemporaryDirectory() as _tmpdir: + runfile = _modify_path(_tmpdir, infile) + args = [runfile.as_posix(), '--verbose', 'debug'] + response = runner.invoke(run, args) + assert response.exit_code == 0 + + response = runner.invoke(collect, args) + assert response.exit_code == 0 diff --git a/junifer/api/tests/test_parser.py b/junifer/api/tests/test_parser.py index a91604d03..a5dcee7a0 100644 --- a/junifer/api/tests/test_parser.py +++ b/junifer/api/tests/test_parser.py @@ -13,6 +13,13 @@ def test_parse_yaml(): with tempfile.TemporaryDirectory() as _tmpdir: fname = Path(_tmpdir) / 'test.yaml' + + with open(fname, 'w') as f: + f.write('foo: bar\n') + contents = parse_yaml(fname) + assert 'foo' in contents + assert 'bar' == contents['foo'] + with open(fname, 'w') as f: f.write('foo: bar\n') f.write('with: numpy\n') -- 2.52.0 From 585dd0fd76392699f3f5d868a98d0c4e72dbe058 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 24 Mar 2022 14:56:10 +0100 Subject: [PATCH 13/40] Renamed run, so delete this file --- junifer/api/run.py | 79 ---------------------------------------------- 1 file changed, 79 deletions(-) delete mode 100644 junifer/api/run.py diff --git a/junifer/api/run.py b/junifer/api/run.py deleted file mode 100644 index f26957272..000000000 --- a/junifer/api/run.py +++ /dev/null @@ -1,79 +0,0 @@ -# Authors: Federico Raimondo -# Leonard Sasse -# License: AGPL - -from pathlib import Path - -from .registry import build -from ..datagrabber.base import BaseDataGrabber -from ..markers.base import BaseMarker -from ..storage.base import BaseFeatureStorage -from ..markers.collection import MarkerCollection - - -def run( - workdir, datagrabber, markers, storage, elements=None): - """Run the pipeline on the selected element - - Parameters - ---------- - workdir : str or path-like object - Directory where the pipeline will be executed - datagrabber : dict - Datagrabber to use. Must have a key 'kind' with the kind of - datagrabber to use. All other keys are passed to the datagrabber - init function. - elements : str, tuple or list[str or tuple] - Element(s) to process. Will be used to index the datagrabber. - markers : list of dict - List of markers to extract. Each marker is a dict with at least two - keys: 'name' and 'kind'. The 'name' key is used to name the output - marker. The 'kind' key is used to specify the kind of marker to - extract. The rest of the keys are used to pass parameters to the - marker calculation. - storage : dict - Storage to use. Must have a key 'kind' with the kind of - storage to use. All other keys are passed to the storage - init function. - """ - datagrabber_params = datagrabber.copy() - datagrabber_kind = datagrabber_params.pop('kind') - storage_params = storage.copy() - storage_kind = storage_params.pop('kind') - - if isinstance(workdir, str): - workdir = Path(workdir) - - datagrabber = build( - 'datagrabber', datagrabber_kind, BaseDataGrabber, - init_params=datagrabber_params) - - built_markers = [] - for t_marker in markers: - kind = t_marker.pop('kind') - t_m = build('marker', kind, BaseMarker, init_params=t_marker) - built_markers.append(t_m) - - storage = build( - 'storage', storage_kind, BaseFeatureStorage, - init_params=storage_params) - - mc = MarkerCollection(built_markers, storage=storage) - - with datagrabber: - if elements is not None: - for t_element in elements: - mc.fit(datagrabber[t_element]) - else: - for t_element in datagrabber: - mc.fit(datagrabber[t_element]) - - -def collect(storage): - storage_params = storage.copy() - storage_kind = storage_params.pop('kind') - - storage = build( - 'storage', storage_kind, BaseFeatureStorage, - init_params=storage_params) - storage.collect() -- 2.52.0 From 02bd4a556e0238e653935b669992d004e2edc870 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 24 Mar 2022 16:23:25 +0100 Subject: [PATCH 14/40] Prototype of junifer queue --- examples/yamls/gmd_mean_htcondor.yaml | 20 +++ junifer/api/cli.py | 69 +++++++-- junifer/api/functions.py | 146 +++++++++++++++++- junifer/api/res/run_conda.sh | 17 ++ junifer/api/tests/data/gmd_mean.yaml | 2 +- junifer/api/tests/data/gmd_mean_htcondor.yaml | 20 +++ junifer_jobs/TestHTCondorQueue/condor.dag | 30 ++++ junifer_jobs/TestHTCondorQueue/condor.submit | 24 +++ junifer_jobs/TestHTCondorQueue/config.yaml | 11 ++ junifer_jobs/TestHTCondorQueue/run_conda.sh | 17 ++ 10 files changed, 333 insertions(+), 23 deletions(-) create mode 100644 examples/yamls/gmd_mean_htcondor.yaml create mode 100644 junifer/api/res/run_conda.sh create mode 100644 junifer/api/tests/data/gmd_mean_htcondor.yaml create mode 100644 junifer_jobs/TestHTCondorQueue/condor.dag create mode 100644 junifer_jobs/TestHTCondorQueue/condor.submit create mode 100644 junifer_jobs/TestHTCondorQueue/config.yaml create mode 100644 junifer_jobs/TestHTCondorQueue/run_conda.sh diff --git a/examples/yamls/gmd_mean_htcondor.yaml b/examples/yamls/gmd_mean_htcondor.yaml new file mode 100644 index 000000000..fddf7aa9b --- /dev/null +++ b/examples/yamls/gmd_mean_htcondor.yaml @@ -0,0 +1,20 @@ +with: junifer.testing.registry +workdir: /tmp + +datagrabber: + kind: OasisVBMTestingDatagrabber +markers: + - name: Schaefer1000x7_Mean + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: mean +storage: + kind: SQLiteFeatureStorage + uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db +queue: + jobname: TestHTCondorQueue + kind: HTCondor + env: + kind: conda + name: junifer + mem: 8G \ No newline at end of file diff --git a/junifer/api/cli.py b/junifer/api/cli.py index c32b21484..5fdc6e276 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -3,7 +3,27 @@ import click from .parser import parse_yaml from .functions import run as api_run from .functions import collect as api_collect -from ..utils.logging import configure_logging +from .functions import queue as api_queue +from ..utils.logging import configure_logging, logger, warn + + +def _parse_elements(element, config): + logger.debug(f'Parsing elements: {element}') + if len(element) == 0: + return None + # TODO: If len == 1, check if its a file, then parse elements from file + elements = [x.split(',') if ',' in x else x for x in element] + logger.debug(f'Parsed elements: {elements}') + if elements is not None and 'elements' in config: + warn('One or more elements have been specified in both the command ' + 'line and in the config file. The command line has precedence ' + 'over the configuration file. That is, the elements specified ' + 'in the command line will be used. The elements specified in ' + 'the configuration file will be ignored. To remove this warning, ' + 'please remove the "elements" item from the configuration file.') + elif elements is None: + elements = config.get('elements', None) + return elements @click.group() @@ -12,37 +32,54 @@ def cli(): @cli.command() -@click.argument('filepath') +@click.argument('filepath', type=click.File('r')) @click.option('-v', '--verbose', type=click.Choice(['warning', 'info', 'debug'], case_sensitive=False), - default='warning') -def run(filepath, verbose): + default='info') +@click.option('--element', type=str, multiple=True) +def run(filepath, element, verbose): configure_logging(level=verbose.upper()) - contents = parse_yaml(filepath) - workdir = contents['workdir'] - datagrabber = contents['datagrabber'] - markers = contents['markers'] - storage = contents['storage'] - elements = contents.get('elements', None) + config = parse_yaml(filepath) + workdir = config['workdir'] + datagrabber = config['datagrabber'] + markers = config['markers'] + storage = config['storage'] + elements = _parse_elements(element, config) api_run( workdir=workdir, datagrabber=datagrabber, markers=markers, storage=storage, elements=elements) @cli.command() -@click.argument('filepath') +@click.argument('filepath', type=click.File('r')) @click.option('-v', '--verbose', type=click.Choice(['warning', 'info', 'debug'], case_sensitive=False), - default='warning') + default='info') def collect(filepath, verbose): configure_logging(level=verbose.upper()) - contents = parse_yaml(filepath) - storage = contents['storage'] + config = parse_yaml(filepath) + storage = config['storage'] api_collect(storage) @cli.command() -def queue(): - click.echo('queue') +@click.argument( + 'filepath', + type=click.Path(exists=True, readable=True, dir_okay=False)) +@click.option('-v', '--verbose', + type=click.Choice(['warning', 'info', 'debug'], + case_sensitive=False), + default='info') +@click.option('--overwrite', is_flag=True) +@click.option('--submit', is_flag=True) +@click.option('--element', type=str, multiple=True) +def queue(filepath, element, overwrite, submit, verbose): + configure_logging(level=verbose.upper()) + config = parse_yaml(filepath) + elements = _parse_elements(element, config) + queue_config = config.pop('queue') + kind = queue_config.pop('kind') + api_queue(config, kind=kind, overwrite=overwrite, submit=submit, + elements=elements, **queue_config) diff --git a/junifer/api/functions.py b/junifer/api/functions.py index b98273051..5311a2a04 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -3,14 +3,27 @@ # License: AGPL from pathlib import Path +import shutil +import yaml +import subprocess from .registry import build +from ..utils import logger, raise_error from ..datagrabber.base import BaseDataGrabber from ..markers.base import BaseMarker from ..storage.base import BaseFeatureStorage from ..markers.collection import MarkerCollection +def _get_datagrabber(datagrabber_config): + datagrabber_params = datagrabber_config.copy() + datagrabber_kind = datagrabber_params.pop('kind') + datagrabber = build( + 'datagrabber', datagrabber_kind, BaseDataGrabber, + init_params=datagrabber_params) + return datagrabber + + def run( workdir, datagrabber, markers, storage, elements=None): """Run the pipeline on the selected element @@ -36,17 +49,13 @@ def run( storage to use. All other keys are passed to the storage init function. """ - datagrabber_params = datagrabber.copy() - datagrabber_kind = datagrabber_params.pop('kind') storage_params = storage.copy() storage_kind = storage_params.pop('kind') if isinstance(workdir, str): workdir = Path(workdir) - datagrabber = build( - 'datagrabber', datagrabber_kind, BaseDataGrabber, - init_params=datagrabber_params) + datagrabber = _get_datagrabber(datagrabber) # Copy to avoid changing the original dict _markers = [x.copy() for x in markers] built_markers = [] @@ -73,8 +82,133 @@ def run( def collect(storage): storage_params = storage.copy() storage_kind = storage_params.pop('kind') - + logger.info(f'Collecting data using {storage_kind}') + logger.debug(f'\tStorage params: {storage_params}') storage = build( 'storage', storage_kind, BaseFeatureStorage, init_params=storage_params) + logger.debug('Running storage.collect()') storage.collect() + logger.info('Collect done') + + +def queue(config, kind, jobname='junifer_job', overwrite=False, elements=None, + **kwargs): + """Queue a job to be executed later + + Parameters + ---------- + kind : str + The kind of job to queue. + **kwargs : dict + The parameters to pass to the job. + """ + # Create a folder within the CWD to store the job files / config + cwd = Path.cwd() + job_dir = cwd / 'junifer_jobs' / jobname + logger.info(f'Creating job in {job_dir.as_posix()}') + if job_dir.exists(): + if overwrite is not True: + raise_error(f'Job folder for {jobname} already exists. ' + 'This error is raise to prevent overwriting files ' + 'of jobs that might be scheduled but yet not ' + 'executed. Either delete the directory ' + f'{job_dir.as_posix()} or set overwrite to True.') + job_dir.mkdir(exist_ok=True, parents=True) + + yaml_config = job_dir / 'config.yaml' + logger.info(f'Writing YAML config to {yaml_config}') + with open(yaml_config, 'w') as f: + f.write(yaml.dump(config)) + + # Get list of elements + if elements is None: + if 'elements' in config: + elements = config['elements'] + else: + # If no elements are specified, use all elements from the + # datagrabber + datagrabber = _get_datagrabber(config['datagrabber']) + with datagrabber as dg: + elements = dg.get_elements() + if kind == 'HTCondor': + _queue_condor(job_dir, yaml_config, elements, **kwargs) + elif kind == 'SLURM': + _queue_slurm(job_dir, yaml_config, elements, **kwargs) + else: + raise ValueError(f'Unknown queue kind: {kind}') + + logger.info('Queue done') + + +def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, + disk='1G', extra_preamble='', verbose='info', submit=False): + logger.debug('Creating HTCondor job') + junifer_args = (f'run {yaml_config.as_posix()} ' + f'--verbose {verbose} --element $(element)') + if env is None: + env = {'kind': 'local'} + if env['kind'] == 'conda': + env_name = env['name'] + executable = 'run_conda.sh' + exec_string = f'{env_name} junifer {junifer_args}' + # TODO: Copy run_conda.sh to job_dir + shutil.copy(Path(__file__).parent / 'res' / executable, + job_dir / executable) + elif env['kind'] == 'venv': + env_name = env['name'] + executable = 'run_venv.sh' + exec_string = f'{env_name} junifer {junifer_args}' + # TODO: Copy run_venv.sh to job_dir + elif env['kind'] == 'local': + executable = 'junifer' + exec_string = junifer_args + else: + raise ValueError(f'Unknown env kind: {env["kind"]}') + log_dir = job_dir / 'logs' + log_dir.mkdir(exist_ok=True, parents=True) + preamble = f""" + # The environment + universe = vanilla + getenv = True + + # Resources + request_cpus = {cpus} + request_memory = {mem} + request_disk = {disk} + + # Executable + initial_dir = {job_dir.as_posix()} + executable = $(initial_dir)/{executable} + transfer_executable = False + + arguments = {exec_string} + + {extra_preamble} + + # Logs + log = {log_dir.as_posix()}/junifer_run_$(element).log + output = {log_dir.as_posix()}/junifer_run_$(element).out + error = {log_dir.as_posix()}/junifer_run_$(element).err + """ + + submit_fname = job_dir / 'condor.submit' + dag_fname = job_dir / 'condor.dag' + with open(submit_fname, 'w') as submit_file: + submit_file.write(preamble) + submit_file.write('queue\n') + + with open(dag_fname, 'w') as dag_file: + # Get all subject and session names from file list + for i_job, t_elem in enumerate(elements): + dag_file.write(f'JOB job{i_job} {submit_fname}\n') + dag_file.write(f'VARS job{i_job} element={t_elem}\n\n') + + if submit: + logger.info('Submitting HTCondor job') + subprocess.run(['condor_submit_dag', dag_fname]) + logger.info('HTCondor job submitted') + + +def _queue_slurm(job_dir, yaml_config, elements): + pass diff --git a/junifer/api/res/run_conda.sh b/junifer/api/res/run_conda.sh new file mode 100644 index 000000000..6f859d050 --- /dev/null +++ b/junifer/api/res/run_conda.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +if [ $# -lt 2 ]; then + echo "This script is ment to run a command within a python environment" + echo "It needs at least 2 parameters." + echo "The first one must be the environment name." + echo "The rest will be the command" + exit -1 +fi + +eval "$(conda shell.bash hook)" +env_name=$1 +echo "Activating ${env_name}" +conda activate $1 +shift 1 +echo "Running ${@} in virtual environment" +$@ \ No newline at end of file diff --git a/junifer/api/tests/data/gmd_mean.yaml b/junifer/api/tests/data/gmd_mean.yaml index af4df1798..c492cc559 100644 --- a/junifer/api/tests/data/gmd_mean.yaml +++ b/junifer/api/tests/data/gmd_mean.yaml @@ -3,7 +3,7 @@ workdir: /tmp datagrabber: kind: OasisVBMTestingDatagrabber -elements: +elements: [1, 2] markers: - name: Schaefer1000x7_Mean kind: ParcelAggregation diff --git a/junifer/api/tests/data/gmd_mean_htcondor.yaml b/junifer/api/tests/data/gmd_mean_htcondor.yaml new file mode 100644 index 000000000..fddf7aa9b --- /dev/null +++ b/junifer/api/tests/data/gmd_mean_htcondor.yaml @@ -0,0 +1,20 @@ +with: junifer.testing.registry +workdir: /tmp + +datagrabber: + kind: OasisVBMTestingDatagrabber +markers: + - name: Schaefer1000x7_Mean + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: mean +storage: + kind: SQLiteFeatureStorage + uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db +queue: + jobname: TestHTCondorQueue + kind: HTCondor + env: + kind: conda + name: junifer + mem: 8G \ No newline at end of file diff --git a/junifer_jobs/TestHTCondorQueue/condor.dag b/junifer_jobs/TestHTCondorQueue/condor.dag new file mode 100644 index 000000000..663280160 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/condor.dag @@ -0,0 +1,30 @@ +JOB job0 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job0 element=1 + +JOB job1 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job1 element=2 + +JOB job2 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job2 element=3 + +JOB job3 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job3 element=4 + +JOB job4 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job4 element=5 + +JOB job5 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job5 element=6 + +JOB job6 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job6 element=7 + +JOB job7 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job7 element=8 + +JOB job8 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job8 element=9 + +JOB job9 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit +VARS job9 element=10 + diff --git a/junifer_jobs/TestHTCondorQueue/condor.submit b/junifer_jobs/TestHTCondorQueue/condor.submit new file mode 100644 index 000000000..48194f711 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/condor.submit @@ -0,0 +1,24 @@ + + # The environment + universe = vanilla + getenv = True + + # Resources + request_cpus = 1 + request_memory = 8G + request_disk = 1G + + # Executable + initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue + executable = $(initial_dir)/run_conda.sh + transfer_executable = False + + arguments = junifer junifer run /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info --element $(element) + + + + # Logs + log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log + output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out + error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err + queue diff --git a/junifer_jobs/TestHTCondorQueue/config.yaml b/junifer_jobs/TestHTCondorQueue/config.yaml new file mode 100644 index 000000000..750062fb0 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/config.yaml @@ -0,0 +1,11 @@ +datagrabber: + kind: OasisVBMTestingDatagrabber +markers: +- atlas: Schaefer1000x7 + kind: ParcelAggregation + method: mean + name: Schaefer1000x7_Mean +storage: + kind: SQLiteFeatureStorage + uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db +workdir: /tmp diff --git a/junifer_jobs/TestHTCondorQueue/run_conda.sh b/junifer_jobs/TestHTCondorQueue/run_conda.sh new file mode 100644 index 000000000..6f859d050 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/run_conda.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +if [ $# -lt 2 ]; then + echo "This script is ment to run a command within a python environment" + echo "It needs at least 2 parameters." + echo "The first one must be the environment name." + echo "The rest will be the command" + exit -1 +fi + +eval "$(conda shell.bash hook)" +env_name=$1 +echo "Activating ${env_name}" +conda activate $1 +shift 1 +echo "Running ${@} in virtual environment" +$@ \ No newline at end of file -- 2.52.0 From d79fbf0aeda9181382855b21088cc34303057657 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 24 Mar 2022 16:24:06 +0100 Subject: [PATCH 15/40] damn flake --- junifer/api/functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/api/functions.py b/junifer/api/functions.py index 5311a2a04..a01a939d1 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -153,7 +153,7 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, executable = 'run_conda.sh' exec_string = f'{env_name} junifer {junifer_args}' # TODO: Copy run_conda.sh to job_dir - shutil.copy(Path(__file__).parent / 'res' / executable, + shutil.copy(Path(__file__).parent / 'res' / executable, job_dir / executable) elif env['kind'] == 'venv': env_name = env['name'] -- 2.52.0 From 8f411c2cc3247be5962782f9c104aee5995a2924 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 09:33:04 +0100 Subject: [PATCH 16/40] Ready to test HTCondor --- junifer/api/functions.py | 79 +++++++++++++++---- junifer/testing.py | 37 --------- junifer/utils/fs.py | 7 ++ junifer_jobs/TestHTCondorQueue/collect.submit | 24 ++++++ junifer_jobs/TestHTCondorQueue/condor.dag | 43 +++++----- junifer_jobs/TestHTCondorQueue/run.submit | 24 ++++++ junifer_jobs/TestHTCondorQueue/run_conda.sh | 0 7 files changed, 141 insertions(+), 73 deletions(-) delete mode 100644 junifer/testing.py create mode 100644 junifer/utils/fs.py create mode 100644 junifer_jobs/TestHTCondorQueue/collect.submit create mode 100644 junifer_jobs/TestHTCondorQueue/run.submit mode change 100644 => 100755 junifer_jobs/TestHTCondorQueue/run_conda.sh diff --git a/junifer/api/functions.py b/junifer/api/functions.py index a01a939d1..5682f55b3 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -9,6 +9,7 @@ import subprocess from .registry import build from ..utils import logger, raise_error +from ..utils.fs import make_executable from ..datagrabber.base import BaseDataGrabber from ..markers.base import BaseMarker from ..storage.base import BaseFeatureStorage @@ -142,32 +143,39 @@ def queue(config, kind, jobname='junifer_job', overwrite=False, elements=None, def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, - disk='1G', extra_preamble='', verbose='info', submit=False): + disk='1G', extra_preamble='', verbose='info', collect=True, + submit=False): logger.debug('Creating HTCondor job') - junifer_args = (f'run {yaml_config.as_posix()} ' - f'--verbose {verbose} --element $(element)') + run_junifer_args = (f'run {yaml_config.as_posix()} ' + f'--verbose {verbose} --element $(element)') + collect_junifer_args = \ + f'collect {yaml_config.as_posix()} --verbose {verbose} ' + + # Set up the env_name, executable and arguments according to the + # environment type if env is None: env = {'kind': 'local'} if env['kind'] == 'conda': env_name = env['name'] executable = 'run_conda.sh' - exec_string = f'{env_name} junifer {junifer_args}' + arguments = f'{env_name} junifer' # TODO: Copy run_conda.sh to job_dir - shutil.copy(Path(__file__).parent / 'res' / executable, - job_dir / executable) + exec_path = job_dir / executable + shutil.copy(Path(__file__).parent / 'res' / executable, exec_path) + make_executable(exec_path) elif env['kind'] == 'venv': env_name = env['name'] executable = 'run_venv.sh' - exec_string = f'{env_name} junifer {junifer_args}' + arguments = f'{env_name} junifer' # TODO: Copy run_venv.sh to job_dir elif env['kind'] == 'local': executable = 'junifer' - exec_string = junifer_args + arguments = '' else: raise ValueError(f'Unknown env kind: {env["kind"]}') log_dir = job_dir / 'logs' log_dir.mkdir(exist_ok=True, parents=True) - preamble = f""" + run_preamble = f""" # The environment universe = vanilla getenv = True @@ -182,7 +190,7 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, executable = $(initial_dir)/{executable} transfer_executable = False - arguments = {exec_string} + arguments = {arguments} {run_junifer_args} {extra_preamble} @@ -192,19 +200,58 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, error = {log_dir.as_posix()}/junifer_run_$(element).err """ - submit_fname = job_dir / 'condor.submit' + submit_run_fname = job_dir / 'run.submit' + submit_collect_fname = job_dir / 'collect.submit' dag_fname = job_dir / 'condor.dag' - with open(submit_fname, 'w') as submit_file: - submit_file.write(preamble) + + # Write to run submit files + with open(submit_run_fname, 'w') as submit_file: + submit_file.write(run_preamble) + submit_file.write('queue\n') + + collect_preamble = f""" + # The environment + universe = vanilla + getenv = True + + # Resources + request_cpus = {cpus} + request_memory = {mem} + request_disk = {disk} + + # Executable + initial_dir = {job_dir.as_posix()} + executable = $(initial_dir)/{executable} + transfer_executable = False + + arguments = {arguments} {collect_junifer_args} + + {extra_preamble} + + # Logs + log = {log_dir.as_posix()}/junifer_collect.log + output = {log_dir.as_posix()}/junifer_collect.out + error = {log_dir.as_posix()}/junifer_collect.err + """ + + # Now create the collect submit file + with open(submit_collect_fname, 'w') as submit_file: + submit_file.write(collect_preamble) # Eval preamble here submit_file.write('queue\n') with open(dag_fname, 'w') as dag_file: # Get all subject and session names from file list for i_job, t_elem in enumerate(elements): - dag_file.write(f'JOB job{i_job} {submit_fname}\n') - dag_file.write(f'VARS job{i_job} element={t_elem}\n\n') + dag_file.write(f'JOB run{i_job} {submit_run_fname}\n') + dag_file.write(f'VARS run{i_job} element={t_elem}\n\n') + if collect is True: + dag_file.write(f'JOB collect {submit_collect_fname}\n') + dag_file.write('PARENT ') + for i_job, t_elem in enumerate(elements): + dag_file.write(f'run{i_job} ') + dag_file.write(f'CHILD collect\n\n') - if submit: + if submit is True: logger.info('Submitting HTCondor job') subprocess.run(['condor_submit_dag', dag_fname]) logger.info('HTCondor job submitted') diff --git a/junifer/testing.py b/junifer/testing.py deleted file mode 100644 index 999f58f69..000000000 --- a/junifer/testing.py +++ /dev/null @@ -1,37 +0,0 @@ -# Authors: Federico Raimondo -# License: AGPL -import tempfile -from nilearn import datasets - -from .datagrabber.base import BaseDataGrabber -from .api.registry import register - - -def register_testing(): - """Register testing datagrabber""" - register( - 'datagrabber', 'OasisVBMTestingDatagrabber', - OasisVBMTestingDatagrabber) - - -class OasisVBMTestingDatagrabber(BaseDataGrabber): - """ - DataGrabber for Oasis VBM testing data. - """ - def __init__(self): - datadir = tempfile.mkdtemp() - types = ['VBM_GM'] - super().__init__(types=types, datadir=datadir) - - def get_elements(self): - return list(range(1, 11)) - - def __getitem__(self, element): - out = {} - out['VBM_GM'] = self._dataset.gray_matter_maps[element - 1] - out['meta'] = {'element': {'subject': element}} - return out - - def __enter__(self): - self._dataset = datasets.fetch_oasis_vbm(n_subjects=10) - return self diff --git a/junifer/utils/fs.py b/junifer/utils/fs.py new file mode 100644 index 000000000..667485bb6 --- /dev/null +++ b/junifer/utils/fs.py @@ -0,0 +1,7 @@ +import os +import stat + + +def make_executable(path): + st = os.stat(path) + os.chmod(path, st.st_mode | stat.S_IEXEC) diff --git a/junifer_jobs/TestHTCondorQueue/collect.submit b/junifer_jobs/TestHTCondorQueue/collect.submit new file mode 100644 index 000000000..abf768d65 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/collect.submit @@ -0,0 +1,24 @@ + + # The environment + universe = vanilla + getenv = True + + # Resources + request_cpus = 1 + request_memory = 8G + request_disk = 1G + + # Executable + initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue + executable = $(initial_dir)/run_conda.sh + transfer_executable = False + + arguments = junifer junifer collect /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info + + + + # Logs + log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log + output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out + error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err + queue diff --git a/junifer_jobs/TestHTCondorQueue/condor.dag b/junifer_jobs/TestHTCondorQueue/condor.dag index 663280160..c99e2889f 100644 --- a/junifer_jobs/TestHTCondorQueue/condor.dag +++ b/junifer_jobs/TestHTCondorQueue/condor.dag @@ -1,30 +1,33 @@ -JOB job0 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job0 element=1 +JOB run0 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run0 element=1 -JOB job1 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job1 element=2 +JOB run1 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run1 element=2 -JOB job2 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job2 element=3 +JOB run2 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run2 element=3 -JOB job3 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job3 element=4 +JOB run3 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run3 element=4 -JOB job4 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job4 element=5 +JOB run4 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run4 element=5 -JOB job5 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job5 element=6 +JOB run5 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run5 element=6 -JOB job6 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job6 element=7 +JOB run6 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run6 element=7 -JOB job7 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job7 element=8 +JOB run7 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run7 element=8 -JOB job8 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job8 element=9 +JOB run8 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run8 element=9 -JOB job9 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/condor.submit -VARS job9 element=10 +JOB run9 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit +VARS run9 element=10 + +JOB collect /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/collect.submit +PARENT run0 run1 run2 run3 run4 run5 run6 run7 run8 run9 CHILD collect diff --git a/junifer_jobs/TestHTCondorQueue/run.submit b/junifer_jobs/TestHTCondorQueue/run.submit new file mode 100644 index 000000000..48194f711 --- /dev/null +++ b/junifer_jobs/TestHTCondorQueue/run.submit @@ -0,0 +1,24 @@ + + # The environment + universe = vanilla + getenv = True + + # Resources + request_cpus = 1 + request_memory = 8G + request_disk = 1G + + # Executable + initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue + executable = $(initial_dir)/run_conda.sh + transfer_executable = False + + arguments = junifer junifer run /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info --element $(element) + + + + # Logs + log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log + output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out + error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err + queue diff --git a/junifer_jobs/TestHTCondorQueue/run_conda.sh b/junifer_jobs/TestHTCondorQueue/run_conda.sh old mode 100644 new mode 100755 -- 2.52.0 From 62a8e3423fb6072adfea15e184c1ecaeff0c3c27 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 10:36:35 +0100 Subject: [PATCH 17/40] Fixes for CI + test in juseless --- junifer/api/cli.py | 8 +++++-- junifer/api/res/run_conda.sh | 10 ++++----- junifer/api/tests/test_cli.py | 28 +++++++++++++++++++++--- junifer/api/tests/test_functions.py | 4 ++-- junifer/markers/tests/test_collection.py | 8 +++---- junifer/testing/datagrabbers.py | 5 +++-- 6 files changed, 45 insertions(+), 18 deletions(-) mode change 100644 => 100755 junifer/api/res/run_conda.sh diff --git a/junifer/api/cli.py b/junifer/api/cli.py index 5fdc6e276..6991ae713 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -32,7 +32,9 @@ def cli(): @cli.command() -@click.argument('filepath', type=click.File('r')) +@click.argument( + 'filepath', + type=click.Path(exists=True, readable=True, dir_okay=False)) @click.option('-v', '--verbose', type=click.Choice(['warning', 'info', 'debug'], case_sensitive=False), @@ -52,7 +54,9 @@ def run(filepath, element, verbose): @cli.command() -@click.argument('filepath', type=click.File('r')) +@click.argument( + 'filepath', + type=click.Path(exists=True, readable=True, dir_okay=False)) @click.option('-v', '--verbose', type=click.Choice(['warning', 'info', 'debug'], case_sensitive=False), diff --git a/junifer/api/res/run_conda.sh b/junifer/api/res/run_conda.sh old mode 100644 new mode 100755 index 6f859d050..f07c86c6f --- a/junifer/api/res/run_conda.sh +++ b/junifer/api/res/run_conda.sh @@ -1,17 +1,17 @@ #!/bin/bash if [ $# -lt 2 ]; then - echo "This script is ment to run a command within a python environment" + echo "This script is meant to run a command within a python environment" echo "It needs at least 2 parameters." echo "The first one must be the environment name." echo "The rest will be the command" - exit -1 + exit 255 fi eval "$(conda shell.bash hook)" env_name=$1 echo "Activating ${env_name}" -conda activate $1 +conda activate "$1" shift 1 -echo "Running ${@} in virtual environment" -$@ \ No newline at end of file +echo "Running ${*} in virtual environment" +"$@" \ No newline at end of file diff --git a/junifer/api/tests/test_cli.py b/junifer/api/tests/test_cli.py index 486393b0c..c29b7262e 100644 --- a/junifer/api/tests/test_cli.py +++ b/junifer/api/tests/test_cli.py @@ -30,9 +30,31 @@ def test_run_collect(): infile = Path(__file__).parent / 'data' / 'gmd_mean.yaml' with tempfile.TemporaryDirectory() as _tmpdir: runfile = _modify_path(_tmpdir, infile) - args = [runfile.as_posix(), '--verbose', 'debug'] - response = runner.invoke(run, args) + run_args = [runfile.as_posix(), '--verbose', 'debug', '--element', + 'sub-01', '--element', 'sub-02', '--element', 'sub-03'] + response = runner.invoke(run, run_args) assert response.exit_code == 0 - response = runner.invoke(collect, args) + # TODO: Check that there are 3 files in the output directory + + collect_args = [runfile.as_posix(), '--verbose', 'debug'] + response = runner.invoke(collect, collect_args) assert response.exit_code == 0 + + # TODO: Check that there are 4 files in the output directory + # TODO: Check that the collected file has the correct number of rows + + run_args = [runfile.as_posix(), '--verbose', 'debug', '--element', + 'sub-01', '--element', 'sub-02', '--element', 'sub-04'] + response = runner.invoke(run, run_args) + assert response.exit_code == 0 + + # TODO: Check that there are 5 files in the output directory + # TODO: Check that the collected file has 3 rows (sub-04 is not there) + + collect_args = [runfile.as_posix(), '--verbose', 'debug'] + response = runner.invoke(collect, collect_args) + assert response.exit_code == 0 + + # TODO: Check that there are 5 files in the output directory + # TODO: Check that the collected file has 4 rows (sub-04 is there) diff --git a/junifer/api/tests/test_functions.py b/junifer/api/tests/test_functions.py index 5fc65510d..ae44675fc 100644 --- a/junifer/api/tests/test_functions.py +++ b/junifer/api/tests/test_functions.py @@ -41,7 +41,7 @@ def test_run(): datagrabber=datagrabber, markers=markers, storage=storage, - elements=[1] + elements=['sub-01'] ) files = list(outdir.glob('*.db')) @@ -52,7 +52,7 @@ def test_run(): datagrabber=datagrabber, markers=markers, storage=storage, - elements=[1, 3] + elements=['sub-01', 'sub-03'] ) files = list(outdir.glob('*.db')) diff --git a/junifer/markers/tests/test_collection.py b/junifer/markers/tests/test_collection.py index 07f601992..bb0bdf154 100644 --- a/junifer/markers/tests/test_collection.py +++ b/junifer/markers/tests/test_collection.py @@ -47,7 +47,7 @@ def test_MarkerCollection(): mc.validate(dg) with dg: - input = dg[1] + input = dg['sub-01'] out = mc.fit(input) assert out is not None assert isinstance(out, dict) @@ -74,7 +74,7 @@ def test_MarkerCollection(): datareader=DefaultDataReader()) assert isinstance(mc2._datareader, DefaultDataReader) with dg: - input = dg[1] + input = dg['sub-01'] out2 = mc2.fit(input) for t_marker in markers: t_name = t_marker.name @@ -106,7 +106,7 @@ def test_MarkerCollection_storage(): mc.validate(dg) assert mc._storage.uri == storage.uri with dg: - input = dg[1] + input = dg['sub-01'] out = mc.fit(input) assert out is None @@ -116,7 +116,7 @@ def test_MarkerCollection_storage(): assert mc2._storage is None with dg: - input = dg[1] + input = dg['sub-01'] out = mc2.fit(input) features = storage.list_features() diff --git a/junifer/testing/datagrabbers.py b/junifer/testing/datagrabbers.py index fba285064..e39511163 100644 --- a/junifer/testing/datagrabbers.py +++ b/junifer/testing/datagrabbers.py @@ -16,11 +16,12 @@ class OasisVBMTestingDatagrabber(BaseDataGrabber): super().__init__(types=types, datadir=datadir) def get_elements(self): - return list(range(1, 11)) + return [f'sub-{x:02d}' for x in list(range(1, 11))] def __getitem__(self, element): out = {} - out['VBM_GM'] = self._dataset.gray_matter_maps[element - 1] + i_sub = int(element.split('-')[1]) - 1 + out['VBM_GM'] = self._dataset.gray_matter_maps[i_sub] out['meta'] = {'element': {'subject': element}} return out -- 2.52.0 From 2239256e517c926805f3b99d534c5773a909be72 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 10:37:43 +0100 Subject: [PATCH 18/40] Remove files not belonging --- junifer_jobs/TestHTCondorQueue/collect.submit | 24 -------------- junifer_jobs/TestHTCondorQueue/condor.dag | 33 ------------------- junifer_jobs/TestHTCondorQueue/condor.submit | 24 -------------- junifer_jobs/TestHTCondorQueue/config.yaml | 11 ------- junifer_jobs/TestHTCondorQueue/run.submit | 24 -------------- junifer_jobs/TestHTCondorQueue/run_conda.sh | 17 ---------- 6 files changed, 133 deletions(-) delete mode 100644 junifer_jobs/TestHTCondorQueue/collect.submit delete mode 100644 junifer_jobs/TestHTCondorQueue/condor.dag delete mode 100644 junifer_jobs/TestHTCondorQueue/condor.submit delete mode 100644 junifer_jobs/TestHTCondorQueue/config.yaml delete mode 100644 junifer_jobs/TestHTCondorQueue/run.submit delete mode 100755 junifer_jobs/TestHTCondorQueue/run_conda.sh diff --git a/junifer_jobs/TestHTCondorQueue/collect.submit b/junifer_jobs/TestHTCondorQueue/collect.submit deleted file mode 100644 index abf768d65..000000000 --- a/junifer_jobs/TestHTCondorQueue/collect.submit +++ /dev/null @@ -1,24 +0,0 @@ - - # The environment - universe = vanilla - getenv = True - - # Resources - request_cpus = 1 - request_memory = 8G - request_disk = 1G - - # Executable - initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue - executable = $(initial_dir)/run_conda.sh - transfer_executable = False - - arguments = junifer junifer collect /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info - - - - # Logs - log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log - output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out - error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err - queue diff --git a/junifer_jobs/TestHTCondorQueue/condor.dag b/junifer_jobs/TestHTCondorQueue/condor.dag deleted file mode 100644 index c99e2889f..000000000 --- a/junifer_jobs/TestHTCondorQueue/condor.dag +++ /dev/null @@ -1,33 +0,0 @@ -JOB run0 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run0 element=1 - -JOB run1 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run1 element=2 - -JOB run2 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run2 element=3 - -JOB run3 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run3 element=4 - -JOB run4 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run4 element=5 - -JOB run5 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run5 element=6 - -JOB run6 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run6 element=7 - -JOB run7 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run7 element=8 - -JOB run8 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run8 element=9 - -JOB run9 /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/run.submit -VARS run9 element=10 - -JOB collect /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/collect.submit -PARENT run0 run1 run2 run3 run4 run5 run6 run7 run8 run9 CHILD collect - diff --git a/junifer_jobs/TestHTCondorQueue/condor.submit b/junifer_jobs/TestHTCondorQueue/condor.submit deleted file mode 100644 index 48194f711..000000000 --- a/junifer_jobs/TestHTCondorQueue/condor.submit +++ /dev/null @@ -1,24 +0,0 @@ - - # The environment - universe = vanilla - getenv = True - - # Resources - request_cpus = 1 - request_memory = 8G - request_disk = 1G - - # Executable - initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue - executable = $(initial_dir)/run_conda.sh - transfer_executable = False - - arguments = junifer junifer run /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info --element $(element) - - - - # Logs - log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log - output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out - error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err - queue diff --git a/junifer_jobs/TestHTCondorQueue/config.yaml b/junifer_jobs/TestHTCondorQueue/config.yaml deleted file mode 100644 index 750062fb0..000000000 --- a/junifer_jobs/TestHTCondorQueue/config.yaml +++ /dev/null @@ -1,11 +0,0 @@ -datagrabber: - kind: OasisVBMTestingDatagrabber -markers: -- atlas: Schaefer1000x7 - kind: ParcelAggregation - method: mean - name: Schaefer1000x7_Mean -storage: - kind: SQLiteFeatureStorage - uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db -workdir: /tmp diff --git a/junifer_jobs/TestHTCondorQueue/run.submit b/junifer_jobs/TestHTCondorQueue/run.submit deleted file mode 100644 index 48194f711..000000000 --- a/junifer_jobs/TestHTCondorQueue/run.submit +++ /dev/null @@ -1,24 +0,0 @@ - - # The environment - universe = vanilla - getenv = True - - # Resources - request_cpus = 1 - request_memory = 8G - request_disk = 1G - - # Executable - initial_dir = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue - executable = $(initial_dir)/run_conda.sh - transfer_executable = False - - arguments = junifer junifer run /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/config.yaml --verbose info --element $(element) - - - - # Logs - log = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).log - output = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).out - error = /Users/fraimondo/dev/tbox/junifer/junifer_jobs/TestHTCondorQueue/logs/junifer_run_$(element).err - queue diff --git a/junifer_jobs/TestHTCondorQueue/run_conda.sh b/junifer_jobs/TestHTCondorQueue/run_conda.sh deleted file mode 100755 index 6f859d050..000000000 --- a/junifer_jobs/TestHTCondorQueue/run_conda.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash - -if [ $# -lt 2 ]; then - echo "This script is ment to run a command within a python environment" - echo "It needs at least 2 parameters." - echo "The first one must be the environment name." - echo "The rest will be the command" - exit -1 -fi - -eval "$(conda shell.bash hook)" -env_name=$1 -echo "Activating ${env_name}" -conda activate $1 -shift 1 -echo "Running ${@} in virtual environment" -$@ \ No newline at end of file -- 2.52.0 From abaa5d2917d5e7f1191250c472512596fbe877ca Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 12:03:55 +0100 Subject: [PATCH 19/40] Fix example + Fix bug with testing datagrabber not using proper met --- examples/yamls/gmd_mean_htcondor.yaml | 2 +- junifer/api/functions.py | 52 +++++++++++++++------------ junifer/api/parser.py | 6 ++-- junifer/datagrabber/base.py | 8 ++--- junifer/storage/base.py | 16 +++++---- junifer/storage/sqlite.py | 28 +++++++++------ junifer/storage/tests/test_sqlite.py | 7 ++++ junifer/testing/datagrabbers.py | 5 +-- 8 files changed, 77 insertions(+), 47 deletions(-) diff --git a/examples/yamls/gmd_mean_htcondor.yaml b/examples/yamls/gmd_mean_htcondor.yaml index fddf7aa9b..fde41b733 100644 --- a/examples/yamls/gmd_mean_htcondor.yaml +++ b/examples/yamls/gmd_mean_htcondor.yaml @@ -10,7 +10,7 @@ markers: method: mean storage: kind: SQLiteFeatureStorage - uri: /Users/fraimondo/dev/tbox/junifer/scratch/db/test.db + uri: /data/group/appliedml/fraimondo/junifer_test/test.db queue: jobname: TestHTCondorQueue kind: HTCondor diff --git a/junifer/api/functions.py b/junifer/api/functions.py index 5682f55b3..e3fb8b80d 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -106,18 +106,22 @@ def queue(config, kind, jobname='junifer_job', overwrite=False, elements=None, """ # Create a folder within the CWD to store the job files / config cwd = Path.cwd() - job_dir = cwd / 'junifer_jobs' / jobname - logger.info(f'Creating job in {job_dir.as_posix()}') - if job_dir.exists(): + jobdir = cwd / 'junifer_jobs' / jobname + logger.info(f'Creating job in {jobdir.as_posix()}') + if jobdir.exists(): if overwrite is not True: raise_error(f'Job folder for {jobname} already exists. ' 'This error is raise to prevent overwriting files ' 'of jobs that might be scheduled but yet not ' 'executed. Either delete the directory ' - f'{job_dir.as_posix()} or set overwrite to True.') - job_dir.mkdir(exist_ok=True, parents=True) + f'{jobdir.as_posix()} or set overwrite to True.') + else: + logger.info( + f'Deleting previous job directory {jobdir.as_posix()}') + shutil.rmtree(jobdir) + jobdir.mkdir(exist_ok=True, parents=True) - yaml_config = job_dir / 'config.yaml' + yaml_config = jobdir / 'config.yaml' logger.info(f'Writing YAML config to {yaml_config}') with open(yaml_config, 'w') as f: f.write(yaml.dump(config)) @@ -133,18 +137,18 @@ def queue(config, kind, jobname='junifer_job', overwrite=False, elements=None, with datagrabber as dg: elements = dg.get_elements() if kind == 'HTCondor': - _queue_condor(job_dir, yaml_config, elements, **kwargs) + _queue_condor(jobname, jobdir, yaml_config, elements, **kwargs) elif kind == 'SLURM': - _queue_slurm(job_dir, yaml_config, elements, **kwargs) + _queue_slurm(jobname, jobdir, yaml_config, elements, **kwargs) else: raise ValueError(f'Unknown queue kind: {kind}') logger.info('Queue done') -def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, - disk='1G', extra_preamble='', verbose='info', collect=True, - submit=False): +def _queue_condor(jobname, jobdir, yaml_config, elements, env, mem='8G', + cpus=1, disk='1G', extra_preamble='', verbose='info', + collect=True, submit=False): logger.debug('Creating HTCondor job') run_junifer_args = (f'run {yaml_config.as_posix()} ' f'--verbose {verbose} --element $(element)') @@ -159,21 +163,21 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, env_name = env['name'] executable = 'run_conda.sh' arguments = f'{env_name} junifer' - # TODO: Copy run_conda.sh to job_dir - exec_path = job_dir / executable + # TODO: Copy run_conda.sh to jobdir + exec_path = jobdir / executable shutil.copy(Path(__file__).parent / 'res' / executable, exec_path) make_executable(exec_path) elif env['kind'] == 'venv': env_name = env['name'] executable = 'run_venv.sh' arguments = f'{env_name} junifer' - # TODO: Copy run_venv.sh to job_dir + # TODO: Copy run_venv.sh to jobdir elif env['kind'] == 'local': executable = 'junifer' arguments = '' else: raise ValueError(f'Unknown env kind: {env["kind"]}') - log_dir = job_dir / 'logs' + log_dir = jobdir / 'logs' log_dir.mkdir(exist_ok=True, parents=True) run_preamble = f""" # The environment @@ -186,7 +190,7 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, request_disk = {disk} # Executable - initial_dir = {job_dir.as_posix()} + initial_dir = {jobdir.as_posix()} executable = $(initial_dir)/{executable} transfer_executable = False @@ -200,9 +204,9 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, error = {log_dir.as_posix()}/junifer_run_$(element).err """ - submit_run_fname = job_dir / 'run.submit' - submit_collect_fname = job_dir / 'collect.submit' - dag_fname = job_dir / 'condor.dag' + submit_run_fname = jobdir / f'run_{jobname}.submit' + submit_collect_fname = jobdir / f'collect_{jobname}.submit' + dag_fname = jobdir / f'{jobname}.dag' # Write to run submit files with open(submit_run_fname, 'w') as submit_file: @@ -220,7 +224,7 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, request_disk = {disk} # Executable - initial_dir = {job_dir.as_posix()} + initial_dir = {jobdir.as_posix()} executable = $(initial_dir)/{executable} transfer_executable = False @@ -243,7 +247,7 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, # Get all subject and session names from file list for i_job, t_elem in enumerate(elements): dag_file.write(f'JOB run{i_job} {submit_run_fname}\n') - dag_file.write(f'VARS run{i_job} element={t_elem}\n\n') + dag_file.write(f'VARS run{i_job} element="{t_elem}"\n\n') if collect is True: dag_file.write(f'JOB collect {submit_collect_fname}\n') dag_file.write('PARENT ') @@ -255,7 +259,11 @@ def _queue_condor(job_dir, yaml_config, elements, env, mem='8G', cpus=1, logger.info('Submitting HTCondor job') subprocess.run(['condor_submit_dag', dag_fname]) logger.info('HTCondor job submitted') + else: + cmd = f'condor_submit_dag {dag_fname.as_posix()}' + logger.info('HTCondor job files created, to submit the job, ' + f'run "{cmd}"') -def _queue_slurm(job_dir, yaml_config, elements): +def _queue_slurm(jobname, jobdir, yaml_config, elements): pass diff --git a/junifer/api/parser.py b/junifer/api/parser.py index 03b041362..a5883ad0a 100644 --- a/junifer/api/parser.py +++ b/junifer/api/parser.py @@ -2,22 +2,24 @@ import yaml import importlib from pathlib import Path -from ..utils.logging import raise_error +from ..utils.logging import raise_error, logger def parse_yaml(filepath): if not isinstance(filepath, Path): filepath = Path(filepath) + logger.info(f'Parsing yaml file: {filepath.as_posix()}') if not filepath.exists(): raise_error(f'File does not exist: {filepath.as_posix()}') with open(filepath, 'r') as f: contents = yaml.safe_load(f) if 'with' in contents: - to_load = contents.pop('with') + to_load = contents['with'] if not isinstance(to_load, list): to_load = [to_load] for t_module in to_load: + logger.info(f'Importing module {t_module}') importlib.import_module(t_module) return contents diff --git a/junifer/datagrabber/base.py b/junifer/datagrabber/base.py index 7df89e8d2..732df21bf 100644 --- a/junifer/datagrabber/base.py +++ b/junifer/datagrabber/base.py @@ -111,9 +111,10 @@ class BaseDataGrabber(ABC): for elem in self.get_elements(): yield elem - @abstractmethod def __getitem__(self, element): - raise NotImplementedError('__getitem__ not implemented') + out = {} + out['meta'] = dict(datagrabber=self.get_meta()) + return out @abstractmethod def get_elements(self): @@ -196,7 +197,7 @@ class BIDSDataGrabber(BaseDataGrabber): Dictionary of paths for each type of data required for the specified element. """ - out = {} + out = super().__getitem__(element) if not isinstance(element, tuple): element = (element,) for t_type in self.types: @@ -208,7 +209,6 @@ class BIDSDataGrabber(BaseDataGrabber): t_out = self.datadir / element[0] / t_replace out[t_type] = dict(path=t_out) # Meta here is element and types - out['meta'] = dict(datagrabber=self.get_meta()) out['meta']['element'] = {'subject': element[0]} if len(element) > 1: out['meta']['element']['session'] = element[1] # type: ignore diff --git a/junifer/storage/base.py b/junifer/storage/base.py index da1f26aa5..dbdf0f204 100644 --- a/junifer/storage/base.py +++ b/junifer/storage/base.py @@ -161,15 +161,19 @@ class BaseFeatureStorage(ABC): raise NotImplementedError('validate_input not implemented') @abstractmethod - def list_features(self): + def list_features(self, return_df=False): """List the features in the storage - + Parameters + ---------- + return_df : bool + If True, return a dataframe. If False, (default) return a + dictionary Returns ------- - features: dict(str, dict) - List of features in the storage. The keys are the feature names - to be used in read_features. The values are the metadata of each - feature + features: dict(str, dict) | pd.DataFrame + List of features in the storage. If dictionarly, the keys are the + feature names to be used in read_features. The values are the + metadata of each feature. """ raise NotImplementedError('list_features not implemented') diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index 9616ff7f6..d063d194c 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -23,14 +23,15 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): Parameters ---------- - uri : str - The connection URI. - Easy options: - 'sqlite://' for an in memory sqlite database - 'sqlite:///' to save in a file - - Check https://docs.sqlalchemy.org/en/14/core/engines.html for more - options + uri : str or Path (must be a file) + The Path to the file to be used. + single_output : bool + If False (default), will create one file per element. The name + of the file will be prefixed with the respective element. + If True, will create only one file, the specified in the URI and + store all the elements in the same file. This behaviour is only + suitable for non-parallel executions. SQLite does not support + concurrency. upsert : str Upsert mode. Options are 'ignore' and 'update' (default). If 'ignore', the existing elements are ignored. If update, the @@ -41,6 +42,10 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): raise ValueError('upsert must be either "update" or "ignore"') if not isinstance(uri, Path): uri = Path(uri) + if not uri.parent.exists(): + logger.info(f'Output directory ({uri.parent.as_posix()}) ' + 'does not exist, creating') + uri.parent.mkdir(parents=True, exist_ok=True) super().__init__(uri, single_output=single_output) self._upsert = upsert self._valid_inputs = ['table', 'timeseries'] @@ -64,10 +69,13 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): uri = f'sqlite:///{self.uri.parent}/{prefix}{self.uri.name}' return create_engine(uri, echo=False) - def list_features(self): + def list_features(self, return_df=False): meta_df = pd.read_sql( 'meta', con=self.get_engine(), index_col='meta_md5') - return meta_df.to_dict(orient='index') + out = meta_df + if return_df is False: + out = meta_df.to_dict(orient='index') + return out def read_df(self, feature_name=None, feature_md5=None): """Read features from the storage. diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index 55e3670e7..a4688497d 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -63,6 +63,13 @@ def test_get_engine(): with pytest.raises(ValueError, match='element must be specified'): storage.get_engine() + tocreate = Path(_tmpdir) / 'tocreate' + assert not tocreate.exists() + uri = f'{tocreate.as_posix()}/test.db' + storage = SQLiteFeatureStorage( + uri=uri, single_output=True, upsert='ignore') + assert tocreate.exists() + def test_store_metadata(): """Test store_metadata""" diff --git a/junifer/testing/datagrabbers.py b/junifer/testing/datagrabbers.py index e39511163..db8003a6f 100644 --- a/junifer/testing/datagrabbers.py +++ b/junifer/testing/datagrabbers.py @@ -19,10 +19,11 @@ class OasisVBMTestingDatagrabber(BaseDataGrabber): return [f'sub-{x:02d}' for x in list(range(1, 11))] def __getitem__(self, element): - out = {} + out = super().__getitem__(element) i_sub = int(element.split('-')[1]) - 1 out['VBM_GM'] = self._dataset.gray_matter_maps[i_sub] - out['meta'] = {'element': {'subject': element}} + # Set the element accordingly + out['meta']['element'] = {'subject': element} return out def __enter__(self): -- 2.52.0 From 98e26780b6069af4ef4c7793241e61f961d7a1fd Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 12:04:26 +0100 Subject: [PATCH 20/40] Add one more TODO --- setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.py b/setup.py index 1c3f673e2..6c76b7b05 100644 --- a/setup.py +++ b/setup.py @@ -25,6 +25,8 @@ def _getversion(): DOWNLOAD_URL = 'https://github.com/juaml/junifer' URL = 'https://juaml.github.io/junifer' +# TODO: Read requirementes from requirements.txt and use them + setuptools.setup( name='junifer', author='Fede Raimondo', -- 2.52.0 From 5746d9ed50d6185b42617acc4e95cc563ac07402 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 12:08:58 +0100 Subject: [PATCH 21/40] Fixed test, ready for demo in Software meeting --- junifer/api/tests/test_parser.py | 3 ++- junifer/datagrabber/tests/test_base_datagrabber.py | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/junifer/api/tests/test_parser.py b/junifer/api/tests/test_parser.py index a5dcee7a0..392a90d7f 100644 --- a/junifer/api/tests/test_parser.py +++ b/junifer/api/tests/test_parser.py @@ -27,7 +27,8 @@ def test_parse_yaml(): contents = parse_yaml(fname) assert 'foo' in contents assert 'bar' == contents['foo'] - assert 'with' not in contents + assert 'with' in contents + assert 'numpy' in contents['with'] assert 'junifer.configs.wrong_config' not in sys.modules diff --git a/junifer/datagrabber/tests/test_base_datagrabber.py b/junifer/datagrabber/tests/test_base_datagrabber.py index a31fa8677..29924744e 100644 --- a/junifer/datagrabber/tests/test_base_datagrabber.py +++ b/junifer/datagrabber/tests/test_base_datagrabber.py @@ -29,8 +29,11 @@ def test_BaseDataGrabber(): return super().get_elements() dg = MyDataGrabber(datadir='/tmp', types=['func']) - with pytest.raises(NotImplementedError): - dg['elem'] + elem = dg['elem'] + assert 'meta' in elem + assert 'datagrabber' in elem['meta'] + assert 'class' in elem['meta']['datagrabber'] + assert MyDataGrabber.__name__ in elem['meta']['datagrabber']['class'] with pytest.raises(NotImplementedError): dg.get_elements() -- 2.52.0 From 6977b0b780cfcaca93356f9deca95103b46279f4 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 25 Mar 2022 14:32:42 +0100 Subject: [PATCH 22/40] One more YAML example + typos + llinter --- examples/yamls/ukb_gmd_mean.yaml | 25 +++++++++++++++++++++++++ junifer/markers/tests/test_parcel.py | 3 ++- junifer/storage/sqlite.py | 2 +- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 examples/yamls/ukb_gmd_mean.yaml diff --git a/examples/yamls/ukb_gmd_mean.yaml b/examples/yamls/ukb_gmd_mean.yaml new file mode 100644 index 000000000..7d2303d7e --- /dev/null +++ b/examples/yamls/ukb_gmd_mean.yaml @@ -0,0 +1,25 @@ +with: junifer.configs.juseless +workdir: /tmp + +datagrabber: + kind: JuselessUKBVBM +elements: +markers: + - name: Schaefer1000x7_TrimMean80 + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: trim_mean + method_params: + proportiontocut: 0.2 + - name: Schaefer1000x7_Mean + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: mean + - name: Schaefer1000x7_Std + kind: ParcelAggregation + atlas: Schaefer1000x7 + method: std +storage: + kind: SQLiteFeatureStorage + uri: /data/project/ukb_motor/junifer_test/test.db + diff --git a/junifer/markers/tests/test_parcel.py b/junifer/markers/tests/test_parcel.py index 7ab06d591..fae7313ab 100644 --- a/junifer/markers/tests/test_parcel.py +++ b/junifer/markers/tests/test_parcel.py @@ -97,7 +97,8 @@ def test_ParcelAggregation_3D(): manual = [] for t_v in sorted(np.unique(atlas_values)): t_values = trim_mean( - data[:, atlas_values == t_v], proportiontocut=0.1, axis=None) + data[:, atlas_values == t_v], proportiontocut=0.1, + axis=None) # type: ignore manual.append(t_values) manual = np.array(manual)[np.newaxis, :] diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index d063d194c..82d5b0590 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -218,7 +218,7 @@ class SQLiteFeatureStorage(PandasFeatureStoreage): Name of the table to save if_exist : str If the table exists, the behavior is controlled by this parameter. - Options are 'append' (default) and 'fail'. If 'fail' and th table + Options are 'append' (default) and 'fail'. If 'fail' and the table exists, it will raise an error. If 'append', the data will be appended to the existing table (following the upsert mode). -- 2.52.0 From f93240da379c4ba19cd011bbeb3c5375e9a2f65e Mon Sep 17 00:00:00 2001 From: LeSasse Date: Mon, 28 Mar 2022 17:26:31 +0200 Subject: [PATCH 23/40] initial not-quite working version of HCP datagrabber --- junifer/datagrabber/hcp.py | 112 +++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 junifer/datagrabber/hcp.py diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py new file mode 100644 index 000000000..0c2a8e940 --- /dev/null +++ b/junifer/datagrabber/hcp.py @@ -0,0 +1,112 @@ +import os +from itertools import product +from ..datagrabber import DataladDataGrabber +from ..api.decorators import register_datagrabber + + +@register_datagrabber +class HCP1200(DataladDataGrabber): + """ Human Connectome Project Datalad DataGrabber class + + Implements a DataGrabber to access the Human Connectome Project + + """ + def __init__(self, datadir=None): + """Initialize a HCP object. + + Parameters + ---------- + datadir : str or Path + That directory where the datalad dataset will be cloned. If None, + (default), the datalad dataset will be cloned into a temporary + directory. + + """ + uri = ( + 'https://github.com/datalad-datasets/' + 'human-connectome-project-openaccess.git' + ) + rootdir = 'HCP1200' + types = ['BOLD', 'T1w'] + super().__init__( + types=types, datadir=datadir, uri=uri, rootdir=rootdir + ) + + def get_elements(self, subjects=None, tasks=None, phase_encodings=None): + """Get the list of subjects in the dataset. + + Returns + ------- + elements : list[str] + The list of subjects in the dataset. + """ + elems = [] + + if isinstance(subjects, str): + subjects = [subjects] + if isinstance(tasks, str): + tasks = [tasks] + if isinstance(phase_encodings, str): + phase_encodings = [phase_encodings] + + if subjects is None: + subjects = os.listdir(self.datadir) + if tasks is None: + tasks = [ + "REST1", + "REST2", + "SOCIAL", + "WM", + "RELATIONAL", + "EMOTION", + "LANGUAGE", + "GAMBLING", + "MOTOR", + ] + if phase_encodings is None: + phase_encodings = ["LR", "RL"] + + for subject, task, phase_encoding in product( + subjects, tasks, phase_encodings + ): + elems.append((subject, task, phase_encoding)) + + return elems + + def __getitem__(self, element): + """Index one element in the dataset. + + Parameters + ---------- + element : tuple[str, str] + The element to be indexed. First element in the tuple is the + subject, second element is the task, third element is the + phase encoding direction. + + Returns + ------- + out : dict[str -> Path] + Dictionary of paths for each type of data required for the + specified element. + """ + sub, task, phase_encoding = element + out = {} + + if "REST" in task: + task_name = f"rfMRI_{task}" + else: + task_name = f"tfMRI_{task}" + + out["BOLD"] = dict( + path=self.datadir / sub / "MNINonLinear" / "Results" / + f"{task_name}_{phase_encoding}" / + f"{task_name}_{phase_encoding}_hp2000_clean.nii.gz" + ) + + self._dataset_get(out) + + out['meta']['element'] = dict( + subject=sub, task=task, phase_encoding=phase_encoding + ) + + return out \ No newline at end of file -- 2.52.0 From 43b290ee86d2604a82dbba12bcadf3ca2be65ce7 Mon Sep 17 00:00:00 2001 From: LeSasse Date: Tue, 29 Mar 2022 09:16:25 +0200 Subject: [PATCH 24/40] moved HCP1200 datagrabber to configs/juseless, because confound files are so far only available on juseless. Updated meta dictionary for JuselessUKBVBM and HCP1200 and updated constructor for HCP1200 --- junifer/configs/juseless.py | 142 ++++++++++++++++++++++++++++++++++++ junifer/datagrabber/hcp.py | 112 ---------------------------- 2 files changed, 142 insertions(+), 112 deletions(-) delete mode 100644 junifer/datagrabber/hcp.py diff --git a/junifer/configs/juseless.py b/junifer/configs/juseless.py index 7bebc030b..ab4f88d17 100644 --- a/junifer/configs/juseless.py +++ b/junifer/configs/juseless.py @@ -1,3 +1,6 @@ +import os +from itertools import product +from pathlib import Path from ..datagrabber import DataladDataGrabber from ..api.decorators import register_datagrabber @@ -61,6 +64,145 @@ class JuselessUKBVBM(DataladDataGrabber): out = {} out['VBM_GM'] = self.datadir / f'm0wp1{sub}_{ses}_T1w.nii.gz' + out['meta'] = dict(datagrabber=self.get_meta()) self._dataset_get(out) out['meta']['element'] = dict(subject=sub, session=ses) return out + + +@register_datagrabber +class HCP1200(DataladDataGrabber): + """ Human Connectome Project Datalad DataGrabber class + + Implements a DataGrabber to access the Human Connectome Project + + """ + + def __init__( + self, datadir=None, subjects=None, tasks=None, phase_encodings=None + ): + """Initialize a HCP object. + + Parameters + ---------- + datadir : str or Path + That directory where the datalad dataset will be cloned. If None, + (default), the datalad dataset will be cloned into a temporary + directory. + subjects : str or list of strings + HCP subject ID's. If 'None' (default), all available subjects are + selected + tasks : str or list of strings + HCP task sessions. If 'None' (default), all available task + sessions are selected. Can be 'REST1', 'REST2', 'SOCIAL', 'WM', + 'RELATIONAL', 'EMOTION', 'LANGUAGE', 'GAMBLING', 'MOTOR', or a + list consisting of these names. + phase_encoding : str or list of strings + HCP phase encoding directions. Can be 'LR' or 'RL'. If 'None' + (default) both will be used. + + """ + uri = ( + 'https://github.com/datalad-datasets/' + 'human-connectome-project-openaccess.git' + ) + rootdir = 'HCP1200' + types = ['BOLD'] + super().__init__( + types=types, datadir=datadir, uri=uri, rootdir=rootdir + ) + + self.subjects = subjects + self.tasks = tasks + self.phase_encodings = phase_encodings + + if isinstance(self.subjects, str): + self.subjects = [self.subjects] + if isinstance(self.tasks, str): + self.tasks = [self.tasks] + if isinstance(self.phase_encodings, str): + self.phase_encodings = [self.phase_encodings] + + if self.tasks is None: + self.tasks = [ + 'REST1', + 'REST2', + 'SOCIAL', + 'WM', + 'RELATIONAL', + 'EMOTION', + 'LANGUAGE', + 'GAMBLING', + 'MOTOR', + ] + + if self.phase_encodings is None: + self.phase_encodings = ["LR", "RL"] + + def get_elements(self): + """Get the list of subjects in the dataset. + + Returns + ------- + elements : list[str] + The list of subjects in the dataset. + """ + elems = [] + + if self.subjects is None: + self.subjects = os.listdir(self.datadir) + + for subject, task, phase_encoding in product( + self.subjects, self.tasks, self.phase_encodings + ): + elems.append((subject, task, phase_encoding)) + + return elems + + def __getitem__(self, element): + """Index one element in the dataset. + + Parameters + ---------- + element : tuple[str, str] + The element to be indexed. First element in the tuple is the + subject, second element is the task, third element is the + phase encoding direction. + + Returns + ------- + out : dict[str -> Path] + Dictionary of paths for each type of data required for the + specified element. + """ + sub, task, phase_encoding = element + out = {} + + if 'REST' in task: + task_name = f'rfMRI_{task}' + else: + task_name = f'tfMRI_{task}' + + out['BOLD'] = dict( + path=self.datadir / sub / 'MNINonLinear' / 'Results' / + f'{task_name}_{phase_encoding}' / + f'{task_name}_{phase_encoding}_hp2000_clean.nii.gz' + ) + + conf_dir = ( + Path('/data') / 'group' / 'appliedml' / + 'data' / 'HCP1200_Confounds_tsv' + ) + out['BOLD']['confounds'] = ( + conf_dir / sub / 'MNINonLinear' / 'Results' / + f'{task_name}_{phase_encoding}' / f'Confounds_{sub}.tsv' + ) + + out['meta'] = dict(datagrabber=self.get_meta()) + self._dataset_get(out) + + out['meta']['element'] = dict( + subject=sub, task=task, phase_encoding=phase_encoding + ) + + return out diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py deleted file mode 100644 index 0c2a8e940..000000000 --- a/junifer/datagrabber/hcp.py +++ /dev/null @@ -1,112 +0,0 @@ -import os -from itertools import product -from ..datagrabber import DataladDataGrabber -from ..api.decorators import register_datagrabber - - -@register_datagrabber -class HCP1200(DataladDataGrabber): - """ Human Connectome Project Datalad DataGrabber class - - Implements a DataGrabber to access the Human Connectome Project - - """ - def __init__(self, datadir=None): - """Initialize a HCP object. - - Parameters - ---------- - datadir : str or Path - That directory where the datalad dataset will be cloned. If None, - (default), the datalad dataset will be cloned into a temporary - directory. - - """ - uri = ( - 'https://github.com/datalad-datasets/' - 'human-connectome-project-openaccess.git' - ) - rootdir = 'HCP1200' - types = ['BOLD', 'T1w'] - super().__init__( - types=types, datadir=datadir, uri=uri, rootdir=rootdir - ) - - def get_elements(self, subjects=None, tasks=None, phase_encodings=None): - """Get the list of subjects in the dataset. - - Returns - ------- - elements : list[str] - The list of subjects in the dataset. - """ - elems = [] - - if isinstance(subjects, str): - subjects = [subjects] - if isinstance(tasks, str): - tasks = [tasks] - if isinstance(phase_encodings, str): - phase_encodings = [phase_encodings] - - if subjects is None: - subjects = os.listdir(self.datadir) - if tasks is None: - tasks = [ - "REST1", - "REST2", - "SOCIAL", - "WM", - "RELATIONAL", - "EMOTION", - "LANGUAGE", - "GAMBLING", - "MOTOR", - ] - if phase_encodings is None: - phase_encodings = ["LR", "RL"] - - for subject, task, phase_encoding in product( - subjects, tasks, phase_encodings - ): - elems.append((subject, task, phase_encoding)) - - return elems - - def __getitem__(self, element): - """Index one element in the dataset. - - Parameters - ---------- - element : tuple[str, str] - The element to be indexed. First element in the tuple is the - subject, second element is the task, third element is the - phase encoding direction. - - Returns - ------- - out : dict[str -> Path] - Dictionary of paths for each type of data required for the - specified element. - """ - sub, task, phase_encoding = element - out = {} - - if "REST" in task: - task_name = f"rfMRI_{task}" - else: - task_name = f"tfMRI_{task}" - - out["BOLD"] = dict( - path=self.datadir / sub / "MNINonLinear" / "Results" / - f"{task_name}_{phase_encoding}" / - f"{task_name}_{phase_encoding}_hp2000_clean.nii.gz" - ) - - self._dataset_get(out) - - out['meta']['element'] = dict( - subject=sub, task=task, phase_encoding=phase_encoding - ) - - return out \ No newline at end of file -- 2.52.0 From 7622c2c6786987371577586fbd364e25fa74b48a Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Tue, 29 Mar 2022 12:08:38 +0200 Subject: [PATCH 25/40] HCP Datagrabber WIP --- .gitignore | 3 +- junifer/configs/juseless.py | 268 ++++++++---------- junifer/configs/tests/test_juseless.py | 9 +- junifer/datagrabber/__init__.py | 3 +- junifer/datagrabber/base.py | 120 +++++--- junifer/datagrabber/hcp.py | 102 +++++++ .../tests/test_base_datagrabber.py | 108 ++++--- junifer/utils/logging.py | 3 +- pyproject.toml | 6 +- 9 files changed, 401 insertions(+), 221 deletions(-) create mode 100644 junifer/datagrabber/hcp.py diff --git a/.gitignore b/.gitignore index 35e6aacdc..94ac529c9 100644 --- a/.gitignore +++ b/.gitignore @@ -133,4 +133,5 @@ cython_debug/ .DS_store junifer/_version.py -scratch/ \ No newline at end of file +scratch/ +junifer_jobs/ \ No newline at end of file diff --git a/junifer/configs/juseless.py b/junifer/configs/juseless.py index ab4f88d17..1918c5d82 100644 --- a/junifer/configs/juseless.py +++ b/junifer/configs/juseless.py @@ -1,13 +1,11 @@ -import os -from itertools import product -from pathlib import Path -from ..datagrabber import DataladDataGrabber +from ..datagrabber import PatternDataladDataGrabber from ..api.decorators import register_datagrabber +from ..utils import logger @register_datagrabber -class JuselessUKBVBM(DataladDataGrabber): - """Juseless UKB VMG DataGrabber class. +class JuselessDataladUKBVBM(PatternDataladDataGrabber): + """Juseless UKB VBM DataGrabber class. Implements a DataGrabber to access the UKB VBM data in Juseless. @@ -26,8 +24,13 @@ class JuselessUKBVBM(DataladDataGrabber): uri = 'ria+http://ukb.ds.inm7.de#~cat_m0wp1' rootdir = 'm0wp1' types = ['VBM_GM'] + replacements = ['subject', 'session'] + patterns = { + 'VBM_GM': 'm0wp1{subject}_{session}_T1w.nii.gz' + } super().__init__( - types=types, datadir=datadir, uri=uri, rootdir=rootdir) + types=types, datadir=datadir, uri=uri, rootdir=rootdir, + replacements=replacements, patterns=patterns) def get_elements(self): """Get the list of subjects in the dataset. @@ -37,6 +40,7 @@ class JuselessUKBVBM(DataladDataGrabber): elements : list[str] The list of subjects in the dataset. """ + logger.debug('Getting the list of subjects in the dataset') elems = [] for x in self.datadir.glob('*._T1w.nii.gz'): sub, ses = x.name.split('_') @@ -45,164 +49,140 @@ class JuselessUKBVBM(DataladDataGrabber): elems.append((sub, ses)) return elems - def __getitem__(self, element): - """Index one element in the dataset. - Parameters - ---------- - element : tuple[str, str] - The element to be indexed. First element in the tuple is the - subject, second element is the session. +# @register_datagrabber +# class HCP1200(PatternDataGrabber): +# """ Human Connectome Project Datalad DataGrabber class - Returns - ------- - out : dict[str -> Path] - Dictionary of paths for each type of data required for the - specified element. - """ - sub, ses = element - out = {} +# Implements a DataGrabber to access the Human Connectome Project - out['VBM_GM'] = self.datadir / f'm0wp1{sub}_{ses}_T1w.nii.gz' - out['meta'] = dict(datagrabber=self.get_meta()) - self._dataset_get(out) - out['meta']['element'] = dict(subject=sub, session=ses) - return out +# """ +# def __init__( +# self, datadir=None, subjects=None, tasks=None, phase_encodings=None +# ): +# """Initialize a HCP object. -@register_datagrabber -class HCP1200(DataladDataGrabber): - """ Human Connectome Project Datalad DataGrabber class +# Parameters +# ---------- +# datadir : str or Path +# That directory where the datalad dataset will be cloned. If None, +# (default), the datalad dataset will be cloned into a temporary +# directory. +# subjects : str or list of strings +# HCP subject ID's. If 'None' (default), all available subjects are +# selected +# tasks : str or list of strings +# HCP task sessions. If 'None' (default), all available task +# sessions are selected. Can be 'REST1', 'REST2', 'SOCIAL', 'WM', +# 'RELATIONAL', 'EMOTION', 'LANGUAGE', 'GAMBLING', 'MOTOR', or a +# list consisting of these names. +# phase_encoding : str or list of strings +# HCP phase encoding directions. Can be 'LR' or 'RL'. If 'None' +# (default) both will be used. - Implements a DataGrabber to access the Human Connectome Project +# """ +# uri = ( +# 'https://github.com/datalad-datasets/' +# 'human-connectome-project-openaccess.git' +# ) +# rootdir = 'HCP1200' +# types = ['BOLD'] +# super().__init__( +# types=types, datadir=datadir, uri=uri, rootdir=rootdir +# ) - """ +# self.subjects = subjects +# self.tasks = tasks +# self.phase_encodings = phase_encodings - def __init__( - self, datadir=None, subjects=None, tasks=None, phase_encodings=None - ): - """Initialize a HCP object. +# if isinstance(self.subjects, str): +# self.subjects = [self.subjects] +# if isinstance(self.tasks, str): +# self.tasks = [self.tasks] +# if isinstance(self.phase_encodings, str): +# self.phase_encodings = [self.phase_encodings] - Parameters - ---------- - datadir : str or Path - That directory where the datalad dataset will be cloned. If None, - (default), the datalad dataset will be cloned into a temporary - directory. - subjects : str or list of strings - HCP subject ID's. If 'None' (default), all available subjects are - selected - tasks : str or list of strings - HCP task sessions. If 'None' (default), all available task - sessions are selected. Can be 'REST1', 'REST2', 'SOCIAL', 'WM', - 'RELATIONAL', 'EMOTION', 'LANGUAGE', 'GAMBLING', 'MOTOR', or a - list consisting of these names. - phase_encoding : str or list of strings - HCP phase encoding directions. Can be 'LR' or 'RL'. If 'None' - (default) both will be used. +# if self.tasks is None: +# self.tasks = [ +# 'REST1', +# 'REST2', +# 'SOCIAL', +# 'WM', +# 'RELATIONAL', +# 'EMOTION', +# 'LANGUAGE', +# 'GAMBLING', +# 'MOTOR', +# ] - """ - uri = ( - 'https://github.com/datalad-datasets/' - 'human-connectome-project-openaccess.git' - ) - rootdir = 'HCP1200' - types = ['BOLD'] - super().__init__( - types=types, datadir=datadir, uri=uri, rootdir=rootdir - ) +# if self.phase_encodings is None: +# self.phase_encodings = ["LR", "RL"] - self.subjects = subjects - self.tasks = tasks - self.phase_encodings = phase_encodings +# def get_elements(self): +# """Get the list of subjects in the dataset. - if isinstance(self.subjects, str): - self.subjects = [self.subjects] - if isinstance(self.tasks, str): - self.tasks = [self.tasks] - if isinstance(self.phase_encodings, str): - self.phase_encodings = [self.phase_encodings] +# Returns +# ------- +# elements : list[str] +# The list of subjects in the dataset. +# """ +# elems = [] - if self.tasks is None: - self.tasks = [ - 'REST1', - 'REST2', - 'SOCIAL', - 'WM', - 'RELATIONAL', - 'EMOTION', - 'LANGUAGE', - 'GAMBLING', - 'MOTOR', - ] +# if self.subjects is None: +# self.subjects = os.listdir(self.datadir) - if self.phase_encodings is None: - self.phase_encodings = ["LR", "RL"] +# for subject, task, phase_encoding in product( +# self.subjects, self.tasks, self.phase_encodings +# ): +# elems.append((subject, task, phase_encoding)) - def get_elements(self): - """Get the list of subjects in the dataset. +# return elems - Returns - ------- - elements : list[str] - The list of subjects in the dataset. - """ - elems = [] +# def __getitem__(self, element): +# """Index one element in the dataset. - if self.subjects is None: - self.subjects = os.listdir(self.datadir) +# Parameters +# ---------- +# element : tuple[str, str] +# The element to be indexed. First element in the tuple is the +# subject, second element is the task, third element is the +# phase encoding direction. - for subject, task, phase_encoding in product( - self.subjects, self.tasks, self.phase_encodings - ): - elems.append((subject, task, phase_encoding)) +# Returns +# ------- +# out : dict[str -> Path] +# Dictionary of paths for each type of data required for the +# specified element. +# """ +# sub, task, phase_encoding = element +# out = {} - return elems +# if 'REST' in task: +# task_name = f'rfMRI_{task}' +# else: +# task_name = f'tfMRI_{task}' - def __getitem__(self, element): - """Index one element in the dataset. +# out['BOLD'] = dict( +# path=self.datadir / sub / 'MNINonLinear' / 'Results' / +# f'{task_name}_{phase_encoding}' / +# f'{task_name}_{phase_encoding}_hp2000_clean.nii.gz' +# ) - Parameters - ---------- - element : tuple[str, str] - The element to be indexed. First element in the tuple is the - subject, second element is the task, third element is the - phase encoding direction. +# conf_dir = ( +# Path('/data') / 'group' / 'appliedml' / +# 'data' / 'HCP1200_Confounds_tsv' +# ) +# out['BOLD']['confounds'] = ( +# conf_dir / sub / 'MNINonLinear' / 'Results' / +# f'{task_name}_{phase_encoding}' / f'Confounds_{sub}.tsv' +# ) - Returns - ------- - out : dict[str -> Path] - Dictionary of paths for each type of data required for the - specified element. - """ - sub, task, phase_encoding = element - out = {} +# out['meta'] = dict(datagrabber=self.get_meta()) +# self._dataset_get(out) - if 'REST' in task: - task_name = f'rfMRI_{task}' - else: - task_name = f'tfMRI_{task}' +# out['meta']['element'] = dict( +# subject=sub, task=task, phase_encoding=phase_encoding +# ) - out['BOLD'] = dict( - path=self.datadir / sub / 'MNINonLinear' / 'Results' / - f'{task_name}_{phase_encoding}' / - f'{task_name}_{phase_encoding}_hp2000_clean.nii.gz' - ) - - conf_dir = ( - Path('/data') / 'group' / 'appliedml' / - 'data' / 'HCP1200_Confounds_tsv' - ) - out['BOLD']['confounds'] = ( - conf_dir / sub / 'MNINonLinear' / 'Results' / - f'{task_name}_{phase_encoding}' / f'Confounds_{sub}.tsv' - ) - - out['meta'] = dict(datagrabber=self.get_meta()) - self._dataset_get(out) - - out['meta']['element'] = dict( - subject=sub, task=task, phase_encoding=phase_encoding - ) - - return out +# return out diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 7c5131327..03a6bb8fc 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -1,14 +1,17 @@ import socket import pytest -from junifer.configs.juseless import JuselessUKBVBM +from junifer.configs.juseless import JuselessDataladUKBVBM +from junifer.utils.logging import configure_logging if socket.gethostname() != 'juseless': pytest.skip('This tests are only for juseless', allow_module_level=True) +configure_logging(level='DEBUG') -def test_juselessukbvbm_datagrabber(): - with JuselessUKBVBM() as dg: + +def test_juselessdataladukbvbm_datagrabber(): + with JuselessDataladUKBVBM() as dg: out = dg[('sub-2670511', 'ses-2')] assert 'VBM_GM' in out assert out['VBM_GM'].name == 'm0wp1sub-2670511_ses-2_T1w.nii.gz' diff --git a/junifer/datagrabber/__init__.py b/junifer/datagrabber/__init__.py index e9866a1be..ce54e9f35 100644 --- a/junifer/datagrabber/__init__.py +++ b/junifer/datagrabber/__init__.py @@ -1,4 +1,5 @@ # Authors: Federico Raimondo # Leonard Sasse # License: AGPL -from .base import BIDSDataladDataGrabber, DataladDataGrabber, BIDSDataGrabber \ No newline at end of file +from .base import (DataladDataGrabber, PatternDataGrabber, + PatternDataladDataGrabber) \ No newline at end of file diff --git a/junifer/datagrabber/base.py b/junifer/datagrabber/base.py index 732df21bf..d99ec3821 100644 --- a/junifer/datagrabber/base.py +++ b/junifer/datagrabber/base.py @@ -8,7 +8,7 @@ import datalad.api as dl from abc import ABC, abstractmethod from ..api.decorators import register_datagrabber -from ..utils.logging import logger, raise_error +from ..utils.logging import logger, raise_error, warn def _validate_types(types): @@ -22,6 +22,22 @@ def _validate_types(types): "types must be a list of strings", TypeError) # type: ignore +def _validate_replacements(replacements, patterns): + """ + Validate the replacements + """ + if not isinstance(replacements, list): + raise_error("replacements must be a list", TypeError) # type: ignore + if any(not isinstance(x, str) for x in replacements): + raise_error( + "replacements must be a list of strings", + TypeError) # type: ignore + + for x in replacements: + if all(f'{{x}}' not in y for y in patterns.values()): + warn(f"Replacement {x} is not part of any pattern") + + def _validate_patterns(types, patterns): """ Validate the patterns. @@ -73,6 +89,9 @@ class BaseDataGrabber(ABC): _validate_types(types) if not isinstance(datadir, Path): datadir = Path(datadir) + logger.debug('Initializing BaseDataGrabber') + logger.debug(f'\t_datadir = {datadir}') + logger.debug(f'\ttypes = {types}') self._datadir = datadir self.types = types @@ -112,13 +131,16 @@ class BaseDataGrabber(ABC): yield elem def __getitem__(self, element): + logger.info(f'Getting element {element}') out = {} out['meta'] = dict(datagrabber=self.get_meta()) return out @abstractmethod def get_elements(self): - raise NotImplementedError('get_elements not implemented') + raise_error( + 'get_elements not implemented', + NotImplementedError) # type: ignore def __enter__(self): return self @@ -128,34 +150,37 @@ class BaseDataGrabber(ABC): @register_datagrabber -class BIDSDataGrabber(BaseDataGrabber): - """BIDS DataGrabber class. Implements a DataGrabber that understands BIDS - database format. +class PatternDataGrabber(BaseDataGrabber): + """Patternd DataGrabber class (abstract). Implements a DataGrabber that + understands patterns to grab data. Attributes ---------- - datadir - types : list + datadir: Path + Directory where the data is stored + types : list[str] List of data types to be grabbed. patterns : dict[str -> str] Patterns for each type of data. + replacements: list[str] + Replacements in the patterns for each item in the `element` tuple. Methods ------- get_elements: list[str] Returns a list of elements that can be grabbed. Each element is a subject in the BIDS database. - __getitem__(str): dict[str -> Path] + __getitem__(str): dict[str -> dict] Returns a dictionary of paths for each type of data required for the specified element. Each occurrence of the string `{subject}` is replaced by the indexed element """ - def __init__(self, types=None, patterns=None, **kwargs): + def __init__(self, types=None, patterns=None, replacements=None, **kwargs): """Initialize a BaseDataGrabber object. Parameters ---------- - types : list of str + types : list[str] The types of data to be grabbed. patterns : dict[str -> str] Patterns for each type of data. The keys are the types and the @@ -165,32 +190,52 @@ class BIDSDataGrabber(BaseDataGrabber): That directory where the data is/will be stored. """ _validate_patterns(types, patterns) + if not isinstance(replacements, list): + replacements = [replacements] + _validate_replacements(replacements, patterns) super().__init__(types=types, **kwargs) + logger.debug('Initializing PatternDataGrabber') + logger.debug(f'\tpatterns = {patterns}') + logger.debug(f'\treplacements = {replacements}') self.patterns = patterns + self.replacements = replacements - def get_elements(self): - """Get all the elements in the BIDS database + def _replace_patterns(self, element, pattern): + """Replace the patterns in the pattern with the element. + + Parameters + ---------- + element : tuple + The element to be used in the replacement. + pattern : str + The pattern to be replaced. Returns ------- - elems : list[str] - List of all the elements in the database root directory + str + The pattern with the element replaced. """ - elems = [x.name for x in self.datadir.iterdir() if x.is_dir()] - return elems + if len(element) != len(self.replacements): + raise_error( + f'The element lenght must be {len(self.replacements)}, ' + f'indicating {self.replacements}') + to_replace = dict(zip(self.replacements, element)) + return pattern.format(**to_replace) def __getitem__(self, element): - """Index one element in the BIDS database. + """Index one element in the database. - Each occurrence of the string `{subject}` is replaced by the indexed - element. + Each occurrence of the strings in `replacements` is replaced by the + corresponding item in the element tuple. Parameters ---------- element : str or tuple The element to be indexed. If one string is provided, it is - assumed to be a subject. If a tuple is provided, it is assumed to - be a (subject, session) pair. + assumed to be a tuple with only one item. If a tuple is provided, + each item in the tuple is the value for the replacement string + specified in `replacements`. + Returns ------- out : dict[str -> Path] @@ -202,24 +247,19 @@ class BIDSDataGrabber(BaseDataGrabber): element = (element,) for t_type in self.types: t_pattern = self.patterns[t_type] # type: ignore - t_replace = t_pattern.replace('{subject}', element[0]) - if len(element) > 1: - t_replace = t_replace.replace( - '{session}', element[1]) # type: ignore + t_replace = self._replace_patterns(element, t_pattern) t_out = self.datadir / element[0] / t_replace out[t_type] = dict(path=t_out) # Meta here is element and types - out['meta']['element'] = {'subject': element[0]} - if len(element) > 1: - out['meta']['element']['session'] = element[1] # type: ignore + out['meta']['element'] = dict(zip(self.replacements, element)) return out @register_datagrabber class DataladDataGrabber(BaseDataGrabber): """ - Datalad DataGrabber class. Implements a DataGrabber that gets data from - a datalad sibling. + Datalad DataGrabber class (abstract). Implements a DataGrabber that gets + data from a datalad sibling. Attributes ---------- @@ -267,6 +307,9 @@ class DataladDataGrabber(BaseDataGrabber): datadir = tempfile.mkdtemp() logger.info(f'datadir set to {datadir}') super().__init__(datadir=datadir, **kwargs) + logger.debug('Initializing DataladDataGrabber') + logger.debug(f'\turi = {uri}') + logger.debug(f'\t_rootdir = {rootdir}') self.uri = uri self._rootdir = rootdir @@ -280,11 +323,15 @@ class DataladDataGrabber(BaseDataGrabber): def install(self): """Install the datalad dataset into the datadir.""" + logger.debug(f'Installing dataset {self.uri} to {self._datadir}') self.dataset = dl.install( # type: ignore self._datadir, source=self.uri) + logger.debug('Dataset installed') def __exit__(self, exc_type, exc_value, exc_traceback): + logger.debug('Removing dataset') self.remove() + logger.debug('Dataset removed') def remove(self): """Remove the datalad dataset from the datadir.""" @@ -293,7 +340,9 @@ class DataladDataGrabber(BaseDataGrabber): def _dataset_get(self, out): for _, v in out.items(): if 'path' in v: + logger.debug(f'Getting {v["path"]}') self.dataset.get(v['path']) + logger.debug(f'Get done') # append the version of the dataset out['meta']['datagrabber']['dataset_commit_id'] = \ @@ -314,15 +363,16 @@ class DataladDataGrabber(BaseDataGrabber): return out -class BIDSDataladDataGrabber(DataladDataGrabber, BIDSDataGrabber): - """BIDS Datalad DataGrabber class. - Implements a DataGrabber that gets data from a datalad sibling which - follows a BIDS format. +class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): + """Pattern-based Datalad DataGrabber class (abstract). + Implements a DataGrabber that gets data from a datalad sibling, + interpreting patterns. + See Also -------- DataladDataGrabber - BIDSDataGrabber + PatternDataGrabber """ def __init__(self, types=None, patterns=None, **kwargs): diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py new file mode 100644 index 000000000..a063fbd16 --- /dev/null +++ b/junifer/datagrabber/hcp.py @@ -0,0 +1,102 @@ +from itertools import product + +from junifer.datagrabber.base import DataladDataGrabber + +from ..datagrabber import PatternDataGrabber +from ..api.decorators import register_datagrabber + + +@register_datagrabber +class HCP1200(PatternDataGrabber): + + def __init__( + self, datadir=None, tasks=None, phase_encodings=None + ): + """Initialize a HCP object. + + Parameters + ---------- + datadir : str or Path + That directory where the datalad dataset will be cloned. If None, + (default), the datalad dataset will be cloned into a temporary + directory. + tasks : str or list of strings + HCP task sessions. If 'None' (default), all available task + sessions are selected. Can be 'REST1', 'REST2', 'SOCIAL', 'WM', + 'RELATIONAL', 'EMOTION', 'LANGUAGE', 'GAMBLING', 'MOTOR', or a + list consisting of these names. + phase_encoding : str or list of strings + HCP phase encoding directions. Can be 'LR' or 'RL'. If 'None' + (default) both will be used. + + """ + + types = ['BOLD'] + # TODO: Validate tasks + # TODO: Validate phase_encodings + + replacements = ['subject', 'task', 'phase_encoding'] + patterns = { + 'BOLD': ('{subject}/MNINonLinear/Results/' + '*fMRI_{task}_{phase_encoding}/' + '*fMRI_{task}_{phase_encoding}_hp2000_clean.nii.gz') + } + super().__init__( + types=types, datadir=datadir, patterns=patterns, + replacements=replacements + ) + + self.tasks = tasks + self.phase_encodings = phase_encodings + + if isinstance(self.tasks, str): + self.tasks = [self.tasks] + if isinstance(self.phase_encodings, str): + self.phase_encodings = [self.phase_encodings] + + if self.tasks is None: + self.tasks = [ + 'REST1', + 'REST2', + 'SOCIAL', + 'WM', + 'RELATIONAL', + 'EMOTION', + 'LANGUAGE', + 'GAMBLING', + 'MOTOR', + ] + + if self.phase_encodings is None: + self.phase_encodings = ["LR", "RL"] + + def get_elements(self): + """Get the list of subjects in the dataset. + + Returns + ------- + elements : list[str] + The list of subjects in the dataset. + """ + + subjects = [x.name for x in self.datadir.iterdir() if x.is_dir()] + elems = [] + for subject, task, phase_encoding in product( + subjects, self.tasks, self.phase_encodings + ): + elems.append((subject, task, phase_encoding)) + + return elems + + +@register_datagrabber +class DataladHCP1200(HCP1200, DataladDataGrabber): + def __init__(self, datadir=None, tasks=None, phase_encodings=None): + uri = ( + 'https://github.com/datalad-datasets/' + 'human-connectome-project-openaccess.git' + ) + rootdir = 'HCP1200' + super().__init__(datadir=datadir, tasks=tasks, + phase_encodings=phase_encodings, + uri=uri, rootdir=rootdir) # type: ignore diff --git a/junifer/datagrabber/tests/test_base_datagrabber.py b/junifer/datagrabber/tests/test_base_datagrabber.py index 29924744e..3b5f2f8f1 100644 --- a/junifer/datagrabber/tests/test_base_datagrabber.py +++ b/junifer/datagrabber/tests/test_base_datagrabber.py @@ -4,8 +4,8 @@ import tempfile import pytest from pathlib import Path -from junifer.datagrabber.base import BIDSDataGrabber, BIDSDataladDataGrabber, \ - BaseDataGrabber +from junifer.datagrabber.base import (PatternDataGrabber, BaseDataGrabber, + PatternDataladDataGrabber) _testing_dataset = { @@ -43,60 +43,98 @@ def test_BaseDataGrabber(): assert dg.types == ['func'] -def test_BIDSDataGrabber(): - """Test BIDSDataGrabber""" +def test_PatternDataGrabber(): + class MyDataGrabber(PatternDataGrabber): + def get_elements(self): + return super().get_elements() + + """Test test_PatternDataGrabber""" with pytest.raises(TypeError, match=r"types must be a list"): - BIDSDataGrabber(datadir='/tmp', types='wrong', - patterns=dict(wrong='pattern')) + MyDataGrabber(datadir='/tmp', types='wrong', + patterns=dict(wrong='pattern'), + replacements='subject') with pytest.raises(TypeError, match=r"must be a list of strings"): - BIDSDataGrabber(datadir='/tmp', types=[1, 2, 3], - patterns={'1': 'pattern', '2': 'pattern', - '3': 'pattern'}) + MyDataGrabber(datadir='/tmp', types=[1, 2, 3], + patterns={'1': 'pattern', '2': 'pattern', + '3': 'pattern'}, + replacements='subject') - datagrabber = BIDSDataGrabber( - datadir='/tmp/data', types=['func', 'anat'], - patterns=dict(func='pattern1', anat='pattern2')) - assert datagrabber.datadir == Path('/tmp/data') - assert datagrabber.types == ['func', 'anat'] - - datagrabber = BIDSDataGrabber( - datadir=Path('/tmp/data'), types=['func', 'anat'], - patterns=dict(func='pattern1', anat='pattern2')) - assert datagrabber.datadir == Path('/tmp/data') - assert datagrabber.types == ['func', 'anat'] + with pytest.raises(ValueError, match=r"must have the same length"): + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns={'1': 'pattern', '2': 'pattern', + '3': 'pattern'}, + replacements=1) with pytest.raises(TypeError, match=r"patterns must be a dict"): - BIDSDataGrabber(datadir='/tmp', types=['func', 'anat'], - patterns='wrong') + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns='wrong', replacements='subject') with pytest.raises(ValueError, match=r"patterns must have the same length"): - BIDSDataGrabber(datadir='/tmp', types=['func', 'anat'], - patterns={'wrong': 'pattern'}) + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns={'wrong': 'pattern'}, replacements='subject') with pytest.raises(ValueError, match=r"patterns must contain all types"): - BIDSDataGrabber(datadir='/tmp', types=['func', 'anat'], - patterns={'wrong': 'pattern', 'func': 'pattern'}) + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns={'wrong': 'pattern', 'func': 'pattern'}, + replacements='subject') + + with pytest.raises(TypeError, match=r"must be a list of strings"): + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns={'func': 'func/test', 'anat': 'anat/test'}, + replacements=1) + + with pytest.warns(RuntimeWarning, match=r"not part of any pattern"): + MyDataGrabber(datadir='/tmp', types=['func', 'anat'], + patterns={'func': 'func/{subject}.nii', + 'anat': 'anat/{subject}.nii'}, + replacements=['subject', 'wrong']) + + datagrabber = MyDataGrabber( + datadir='/tmp/data', types=['func', 'anat'], + patterns={'func': 'func/{subject}.nii', + 'anat': 'anat/{subject}.nii'}, + replacements='subject') + assert datagrabber.datadir == Path('/tmp/data') + assert datagrabber.types == ['func', 'anat'] + assert datagrabber.replacements == ['subject'] + + datagrabber = MyDataGrabber( + datadir=Path('/tmp/data'), types=['func', 'anat'], + patterns={'func': 'func/{subject}.nii', + 'anat': 'anat/{subject}_{session}.nii'}, + replacements=['subject', 'session']) + assert datagrabber.datadir == Path('/tmp/data') + assert datagrabber.types == ['func', 'anat'] + assert datagrabber.replacements == ['subject', 'session'] -def test_BIDSDataladDataGrabber(): - """Test BIDSDataladDataGrabber""" +def test_bids_datalad_PatternDataGrabber(): + """Test a subject-based BIDS datalad datagrabber""" types = ['T1w', 'bold'] patterns = { 'T1w': 'anat/{subject}_T1w.nii.gz', 'bold': 'func/{subject}_task-rest_bold.nii.gz' } + replacements = ['subject'] + + class MyDataGrabber(PatternDataladDataGrabber): + def get_elements(self): + elems = [x.name for x in self.datadir.iterdir() if x.is_dir()] + return elems with pytest.raises(ValueError, match=r"uri must be provided"): - BIDSDataladDataGrabber(datadir=None, types=types, patterns=patterns) + MyDataGrabber(datadir=None, types=types, patterns=patterns, + replacements=replacements) repo_uri = _testing_dataset['example_bids']['uri'] rootdir = 'example_bids' repo_commit = _testing_dataset['example_bids']['id'] - with BIDSDataladDataGrabber(rootdir=rootdir, uri=repo_uri, - types=types, patterns=patterns) as dg: + with MyDataGrabber(rootdir=rootdir, uri=repo_uri, + types=types, patterns=patterns, + replacements=replacements) as dg: subs = [x for x in dg] expected_subs = [f'sub-{i:02d}' for i in range(1, 10)] assert set(subs) == set(expected_subs) @@ -114,7 +152,7 @@ def test_BIDSDataladDataGrabber(): assert 'datagrabber' in t_sub['meta'] dg_meta = t_sub['meta']['datagrabber'] assert 'class' in dg_meta - assert dg_meta['class'] == 'BIDSDataladDataGrabber' + assert dg_meta['class'] == 'MyDataGrabber' assert 'uri' in dg_meta assert dg_meta['uri'] == repo_uri assert 'dataset_commit_id' in dg_meta @@ -125,7 +163,7 @@ def test_BIDSDataladDataGrabber(): with tempfile.TemporaryDirectory() as tmpdir: datadir = Path(tmpdir) / 'dataset' # Need this for testing - with BIDSDataladDataGrabber(rootdir=rootdir, uri=repo_uri, - types=types, patterns=patterns, - datadir=datadir) as dg: + with MyDataGrabber(rootdir=rootdir, uri=repo_uri, + types=types, patterns=patterns, + datadir=datadir, replacements=replacements) as dg: assert dg.datadir == datadir / rootdir diff --git a/junifer/utils/logging.py b/junifer/utils/logging.py index a5558aad8..343ae4954 100644 --- a/junifer/utils/logging.py +++ b/junifer/utils/logging.py @@ -126,7 +126,8 @@ def configure_logging(level='WARNING', fname=None, overwrite=None, """ _close_handlers(logger) if output_format is None: - output_format = '%(asctime)s - %(name)s - %(levelname)s - %(message)s' + output_format = ('%(asctime)s [%(levelname)8s] %(message)s ' + '(%(filename)s:%(lineno)s)') formatter = logging.Formatter(output_format) if fname is not None: diff --git a/pyproject.toml b/pyproject.toml index 9cae268d7..22429039a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,4 +6,8 @@ build-backend = "setuptools.build_meta" version_scheme = "python-simplified-semver" local_scheme = "no-local-version" write_to = "junifer/_version.py" -write_to_template = "__version__ = '{version}'\n" \ No newline at end of file +write_to_template = "__version__ = '{version}'\n" + +[tool.pytest.ini_options] +log_cli = true +log_cli_level = "WARNING" \ No newline at end of file -- 2.52.0 From ce73351bb8a3185968be4ca9145c67e7eacc8689 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Tue, 29 Mar 2022 13:17:44 +0200 Subject: [PATCH 26/40] Fix pattern data grabber: add option to use '*' --- junifer/datagrabber/base.py | 12 +++++++++++- .../datagrabber/tests/test_base_datagrabber.py | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/junifer/datagrabber/base.py b/junifer/datagrabber/base.py index d99ec3821..a7c97084f 100644 --- a/junifer/datagrabber/base.py +++ b/junifer/datagrabber/base.py @@ -248,7 +248,17 @@ class PatternDataGrabber(BaseDataGrabber): for t_type in self.types: t_pattern = self.patterns[t_type] # type: ignore t_replace = self._replace_patterns(element, t_pattern) - t_out = self.datadir / element[0] / t_replace + if '*' in t_replace: + t_matches = list(self.datadir.glob(t_replace)) + if len(t_matches) > 1: + raise_error( + f'More than one file matches for {element} / {t_type}: ' + f'{t_matches}') + elif len(t_matches) == 0: + raise_error(f'No file matches for {element} / {t_type}') + t_out = t_matches[0] + else: + t_out = self.datadir / t_replace out[t_type] = dict(path=t_out) # Meta here is element and types out['meta']['element'] = dict(zip(self.replacements, element)) diff --git a/junifer/datagrabber/tests/test_base_datagrabber.py b/junifer/datagrabber/tests/test_base_datagrabber.py index 3b5f2f8f1..2a4df25d7 100644 --- a/junifer/datagrabber/tests/test_base_datagrabber.py +++ b/junifer/datagrabber/tests/test_base_datagrabber.py @@ -114,8 +114,8 @@ def test_bids_datalad_PatternDataGrabber(): """Test a subject-based BIDS datalad datagrabber""" types = ['T1w', 'bold'] patterns = { - 'T1w': 'anat/{subject}_T1w.nii.gz', - 'bold': 'func/{subject}_task-rest_bold.nii.gz' + 'T1w': '{subject}/anat/{subject}_T1w.nii.gz', + 'bold': '{subject}/func/{subject}_task-rest_bold.nii.gz' } replacements = ['subject'] @@ -163,7 +163,19 @@ def test_bids_datalad_PatternDataGrabber(): with tempfile.TemporaryDirectory() as tmpdir: datadir = Path(tmpdir) / 'dataset' # Need this for testing + patterns = { + 'T1w': '{subject}/anat/{subject}_T*w.nii.gz', + 'bold': '{subject}/func/{subject}_task-rest_*.nii.gz' + } with MyDataGrabber(rootdir=rootdir, uri=repo_uri, types=types, patterns=patterns, datadir=datadir, replacements=replacements) as dg: assert dg.datadir == datadir / rootdir + for elem in dg: + t_sub = dg[elem] + assert 'path' in t_sub['T1w'] + assert t_sub['T1w']['path'] == \ + (dg.datadir / f'{elem}/anat/{elem}_T1w.nii.gz') + assert 'path' in t_sub['bold'] + assert t_sub['bold']['path'] == \ + (dg.datadir / f'{elem}/func/{elem}_task-rest_bold.nii.gz') -- 2.52.0 From 7106f3d02cd958ed2995d77cf19a677d8fe06189 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Tue, 29 Mar 2022 13:18:25 +0200 Subject: [PATCH 27/40] flake --- junifer/datagrabber/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/junifer/datagrabber/base.py b/junifer/datagrabber/base.py index a7c97084f..1e8ac9c8c 100644 --- a/junifer/datagrabber/base.py +++ b/junifer/datagrabber/base.py @@ -252,8 +252,8 @@ class PatternDataGrabber(BaseDataGrabber): t_matches = list(self.datadir.glob(t_replace)) if len(t_matches) > 1: raise_error( - f'More than one file matches for {element} / {t_type}: ' - f'{t_matches}') + f'More than one file matches for {element} / {t_type}:' + f' {t_matches}') elif len(t_matches) == 0: raise_error(f'No file matches for {element} / {t_type}') t_out = t_matches[0] -- 2.52.0 From f3c1bd1e3e9488e7443b85554e5d2f73994c81b6 Mon Sep 17 00:00:00 2001 From: LeSasse Date: Tue, 29 Mar 2022 16:25:05 +0200 Subject: [PATCH 28/40] updated HCP1200 pattern and __getitem__() method and added test for juseless --- junifer/configs/tests/test_juseless.py | 13 ++++++ junifer/datagrabber/hcp.py | 56 ++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 03a6bb8fc..894eaf793 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -1,8 +1,10 @@ import socket import pytest +import os from junifer.configs.juseless import JuselessDataladUKBVBM from junifer.utils.logging import configure_logging +from junifer.datagrabber.hcp import DataladHCP1200 if socket.gethostname() != 'juseless': pytest.skip('This tests are only for juseless', allow_module_level=True) @@ -16,3 +18,14 @@ def test_juselessdataladukbvbm_datagrabber(): assert 'VBM_GM' in out assert out['VBM_GM'].name == 'm0wp1sub-2670511_ses-2_T1w.nii.gz' assert out['VBM_GM'].exists() + + +def test_juselessdataladhcp_datagrabber(): + with DataladHCP1200() as dg: + all_elements = dg.get_elements() + test_element = all_elements[0] + + out = dg[test_element] + + assert out['BOLD'].exists() + assert os.path.isfile(out['BOLD']['path']) diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index a063fbd16..5336f1ab9 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -38,8 +38,8 @@ class HCP1200(PatternDataGrabber): replacements = ['subject', 'task', 'phase_encoding'] patterns = { 'BOLD': ('{subject}/MNINonLinear/Results/' - '*fMRI_{task}_{phase_encoding}/' - '*fMRI_{task}_{phase_encoding}_hp2000_clean.nii.gz') + '{task}_{phase_encoding}/' + '{task}_{phase_encoding}_hp2000_clean.nii.gz') } super().__init__( types=types, datadir=datadir, patterns=patterns, @@ -68,7 +68,16 @@ class HCP1200(PatternDataGrabber): ] if self.phase_encodings is None: - self.phase_encodings = ["LR", "RL"] + self.phase_encodings = ['LR', 'RL'] + + repl_tasks = [] + for t in self.tasks: + if 'REST' in t: + repl_tasks.append(f'rfMRI_{t}') + else: + repl_tasks.append(f'tfMRI_{t}') + + self.tasks = repl_tasks def get_elements(self): """Get the list of subjects in the dataset. @@ -88,9 +97,48 @@ class HCP1200(PatternDataGrabber): return elems + def __getitem__(self, element): + """Index one element in the dataset. + + Parameters + ---------- + element : tuple[str, str] + The element to be indexed. First element in the tuple is the + subject, second element is the task, third element is the + phase encoding direction. + + Returns + ------- + out : dict[str -> Path] + Dictionary of paths for each type of data required for the + specified element. + """ + + sub, task, phase_encoding = element + + out = super().__getitem__(element) + + self.tasks = [x.split("_")[1] for x in self.tasks] + + out['meta'] = dict(datagrabber=self.get_meta()) + out['meta']['element'] = dict( + subject=sub, task=task, phase_encoding=phase_encoding + ) + + repl_tasks = [] + for t in self.tasks: + if 'REST' in t: + repl_tasks.append(f'rfMRI_{t}') + else: + repl_tasks.append(f'tfMRI_{t}') + + self.tasks = repl_tasks + + return out + @register_datagrabber -class DataladHCP1200(HCP1200, DataladDataGrabber): +class DataladHCP1200(DataladDataGrabber, HCP1200,): def __init__(self, datadir=None, tasks=None, phase_encodings=None): uri = ( 'https://github.com/datalad-datasets/' -- 2.52.0 From 7026ba71927485e44b49ea56d552ce7631e027ef Mon Sep 17 00:00:00 2001 From: LeSasse Date: Tue, 29 Mar 2022 16:38:11 +0200 Subject: [PATCH 29/40] improved __getitem__() --- junifer/datagrabber/hcp.py | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index 5336f1ab9..decf94873 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -70,15 +70,6 @@ class HCP1200(PatternDataGrabber): if self.phase_encodings is None: self.phase_encodings = ['LR', 'RL'] - repl_tasks = [] - for t in self.tasks: - if 'REST' in t: - repl_tasks.append(f'rfMRI_{t}') - else: - repl_tasks.append(f'tfMRI_{t}') - - self.tasks = repl_tasks - def get_elements(self): """Get the list of subjects in the dataset. @@ -116,24 +107,16 @@ class HCP1200(PatternDataGrabber): sub, task, phase_encoding = element - out = super().__getitem__(element) + if "REST" in task: + new_task = f'rfMRI_{task}' + else: + new_task = f'tfMRI_{task}' - self.tasks = [x.split("_")[1] for x in self.tasks] - - out['meta'] = dict(datagrabber=self.get_meta()) + out = super().__getitem__((sub, new_task, phase_encoding)) out['meta']['element'] = dict( subject=sub, task=task, phase_encoding=phase_encoding ) - repl_tasks = [] - for t in self.tasks: - if 'REST' in t: - repl_tasks.append(f'rfMRI_{t}') - else: - repl_tasks.append(f'tfMRI_{t}') - - self.tasks = repl_tasks - return out -- 2.52.0 From 81c2e1bf1785b70b62d86aa5b7258d513b054743 Mon Sep 17 00:00:00 2001 From: LeSasse Date: Wed, 30 Mar 2022 12:11:48 +0200 Subject: [PATCH 30/40] validating HCP datagrabber tasks and phase encodings, also added initial confound removal module --- junifer/datagrabber/hcp.py | 43 +++++--- junifer/preprocess/confounds.py | 169 ++++++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 14 deletions(-) create mode 100644 junifer/preprocess/confounds.py diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index decf94873..f3ef35215 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -32,8 +32,6 @@ class HCP1200(PatternDataGrabber): """ types = ['BOLD'] - # TODO: Validate tasks - # TODO: Validate phase_encodings replacements = ['subject', 'task', 'phase_encoding'] patterns = { @@ -54,22 +52,39 @@ class HCP1200(PatternDataGrabber): if isinstance(self.phase_encodings, str): self.phase_encodings = [self.phase_encodings] + all_tasks = [ + 'REST1', + 'REST2', + 'SOCIAL', + 'WM', + 'RELATIONAL', + 'EMOTION', + 'LANGUAGE', + 'GAMBLING', + 'MOTOR', + ] + if self.tasks is None: - self.tasks = [ - 'REST1', - 'REST2', - 'SOCIAL', - 'WM', - 'RELATIONAL', - 'EMOTION', - 'LANGUAGE', - 'GAMBLING', - 'MOTOR', - ] + self.tasks = all_tasks if self.phase_encodings is None: self.phase_encodings = ['LR', 'RL'] + for task in self.tasks: + if task not in all_tasks: + raise ValueError( + f'{task} not a valid HCP-YA fMRI task input! \n' + f'task can be any of {all_tasks}' + ) + + for pe in self.phase_encodings: + if pe not in ['LR', 'RL']: + raise ValueError( + f'{pe} not a valid HCP-YA phase encoding. \n' + 'phase_encoding can be LR or RL (or both)!' + ) + + def get_elements(self): """Get the list of subjects in the dataset. @@ -121,7 +136,7 @@ class HCP1200(PatternDataGrabber): @register_datagrabber -class DataladHCP1200(DataladDataGrabber, HCP1200,): +class DataladHCP1200(DataladDataGrabber, HCP1200): def __init__(self, datadir=None, tasks=None, phase_encodings=None): uri = ( 'https://github.com/datalad-datasets/' diff --git a/junifer/preprocess/confounds.py b/junifer/preprocess/confounds.py new file mode 100644 index 000000000..5504f5cdf --- /dev/null +++ b/junifer/preprocess/confounds.py @@ -0,0 +1,169 @@ +import numpy as np + +from junifer.markers.base import PipelineStepMixin + +class BaseConfoundRemover(PipelineStepMixin): + """ A base class to read confound files and select columns according to + a pre-defined strategy + + """ + + # TODO: Properly Validate Input + # TODO: implement get_output_kind + # TODO: implement fit_transform + + # lower priority + # TODO: implement more strategies from + # nilearn.interfaces.fmriprep.load_confounds for Felix's confound files, + # in particular scrubbing + # TODO: Implement read_confounds for fmriprep data + + def __init__(self, strategy): + """ Initialise a BaseConfoundReader object + + Parameters + ----------- + strategy : dict + keys of dictionary should correspond to names of noise components + to include: + - 'motion' + - 'wm_csf' + - 'global_signal' + values of dictionary should correspond to types of confounds + extracted from each signal: + - 'basic' - only the confounding time series + - 'power2' - signal + quadratic term + - 'derivatives' - signal + derivatives + - 'full' - signal + deriv. + quadratic terms + power2 deriv. + """ + self.strategy = strategy + + + def validate_input(self, input): + """Validate the input to the pipeline step. + + Parameters + ---------- + input : Junifer Data dictionary + The input to the pipeline step. + + Raises + ------ + ValueError: + If the input does not have the required data. + """ + for k, v in self.strategy.items(): + if k not in ['motion','wm_csf','global_signal']: + raise ValueError( + f'{k} not a valid noise component to include!\n' + f'If {k} is a valid parameter in ' + 'nilearn.interfaces.fmriprep.load_confounds we may ' + 'include it in the future' + ) + if v not in ['basic','power2','derivatives','full']: + raise ValueError( + f'{v} not a valid type of confound to extract from input ' + 'signal!' + ) + + + def get_output_kind(self, input): + """Get the kind of the pipeline step. + + Parameters + ---------- + input : Junifer Data dictionary + The input to the pipeline step. + + Returns + ------- + output : Junifer Data dictionary + The output of the pipeline step. + """ + raise NotImplementedError('get_output_kind not implemented') + + def read_confounds(self): + raise NotImplementedError('read_confounds not implemented') + + def remove_confounds(self): + pass + + def fit_transform(self, input): + out = {} + meta = input.get('meta', {}) + bold_img = input['BOLD']['data'] + confounds_df = input['confounds'] + selected_confounds = self.read_confounds(confounds_df) + clean_img = self.remove_confounds(bold_img, confounds_df) + + + return out + +class FelixConfoundRemover(BaseConfoundRemover): + + def read_confounds(self, confound_dataframe): + + confounds_to_select = [] + # for some confounds we need to manually calculate derivatives using + # numpy and add them to the output + derivatives_to_calculate = [] + + for comp, param in self.strategy.items(): + if comp == 'motion': + + # there should be six rigid body parameters + for i in range(1,7): + # select basic + confounds_to_select.append(f'RP.{i}') + + # select squares + if param in ['power2', 'full']: + confounds_to_select.append(f'RP^2.{i}') + + # select derivatives + if param in ['derivatives', 'full']: + confounds_to_select.append(f'DRP.{i}') + + # if 'full' we should not forget the derivative + # of the squares + if param in ['full']: + confounds_to_select.append(f'DRP^2.{i}') + + elif comp == 'wm_csf': + confounds = ['WM', 'CSF'] + elif comp == 'global_signal': + confounds = ['GS'] + + for conf in confounds: + + confounds_to_select.append(conf) + + # select squares + if param in ['power2', 'full']: + confounds_to_select.append(f'{conf}^2') + + # we have to calculate derivatives (not included in felix' + # confound files) + if param in ['derivatives', 'full']: + derivatives_to_calculate.append(conf) + + if param in ['full']: + derivatives_to_calculate.append(f'{conf}^2') + + confounds_to_remove = confound_dataframe[confounds_to_select] + + for conf in derivatives_to_calculate: + confounds_to_remove[f'D{conf}'] = np.append( + np.diff(confound_dataframe[conf]), 0 + ) + + return confounds_to_remove + +class FmriprepConfoundRemover(BaseConfoundRemover): + """ A ConfoundRemover class for fmriprep output utilising + nilearn's nilearn.interfaces.fmriprep.load_confounds + """ + + def read_confounds(self): + raise NotImplementedError('read_confounds not implemented') + \ No newline at end of file -- 2.52.0 From 8bcc54abc6ee8ba5b1e67120138fcd223e228c71 Mon Sep 17 00:00:00 2001 From: LeSasse Date: Thu, 31 Mar 2022 09:10:22 +0200 Subject: [PATCH 31/40] completed first version of FelixConfoundRemover and added tests --- junifer/preprocess/confounds.py | 232 ++++++++++++++++----- junifer/preprocess/tests/test_confounds.py | 140 +++++++++++++ 2 files changed, 324 insertions(+), 48 deletions(-) create mode 100644 junifer/preprocess/tests/test_confounds.py diff --git a/junifer/preprocess/confounds.py b/junifer/preprocess/confounds.py index 5504f5cdf..774dc36a1 100644 --- a/junifer/preprocess/confounds.py +++ b/junifer/preprocess/confounds.py @@ -1,26 +1,49 @@ +# Authors: Federico Raimondo +# Leonard Sasse +# License: AGPL + import numpy as np +import pandas as pd + +from nilearn._utils.niimg_conversions import check_niimg_4d +from nilearn.masking import compute_brain_mask +from nilearn.image import clean_img from junifer.markers.base import PipelineStepMixin +from ..utils.logging import logger, raise_error + class BaseConfoundRemover(PipelineStepMixin): - """ A base class to read confound files and select columns according to + """ A base class to read confound files and select columns according to a pre-defined strategy - + """ - # TODO: Properly Validate Input - # TODO: implement get_output_kind - # TODO: implement fit_transform - # lower priority - # TODO: implement more strategies from + # TODO: implement more strategies from # nilearn.interfaces.fmriprep.load_confounds for Felix's confound files, # in particular scrubbing # TODO: Implement read_confounds for fmriprep data - def __init__(self, strategy): + def __init__( + self, + strategy={ + 'motion': 'full', + 'wm_csf': 'full', + 'global_signal': 'full' + }, + spike=None, + detrend=True, + standardize=True, + low_pass=None, + high_pass=None, + t_r=None, + mask_img=None + ): """ Initialise a BaseConfoundReader object - + + Confound removal is based on nilearn.image.clean_img + Parameters ----------- strategy : dict @@ -29,15 +52,58 @@ class BaseConfoundRemover(PipelineStepMixin): - 'motion' - 'wm_csf' - 'global_signal' - values of dictionary should correspond to types of confounds + values of dictionary should correspond to types of confounds extracted from each signal: - 'basic' - only the confounding time series - 'power2' - signal + quadratic term - 'derivatives' - signal + derivatives - 'full' - signal + deriv. + quadratic terms + power2 deriv. - """ - self.strategy = strategy + spike : None (default) | float + If None, no spike regressor is added. If spike is a float, it will + add a spike regressor for every point at which FD exceeds the + specified float. + detrend : bool + If detrending should be applied on timeseries + (before confound removal). Default=True. + standardize : bool + If True, returned signals are set to unit variance. Default=True. + low_pass : float + Low cutoff frequencies, in Hertz. + high_pass : float + High cutoff frequencies, in Hertz. + t_r : float + Repetition time, in second (sampling period). + If set to None it will use t_r from nifti header. + mask_img: Niimg-like object + If provided, signal is only cleaned from voxels inside the mask. + If mask is provided, it should have same shape and affine as imgs. + If not provided, a mask is computed using + nilearn.masking.compute_brain_mask + """ + + self.strategy = strategy + self.spike = spike + self.detrend = detrend + self.standardize = standardize + self.low_pass = low_pass + self.high_pass = high_pass + self.t_r = t_r + self.mask_img = mask_img + + for k, v in self.strategy.items(): + if k not in ['motion', 'wm_csf', 'global_signal']: + raise_error( + f'{k} not a valid noise component to include!\n' + f'If {k} is a valid parameter in ' + 'nilearn.interfaces.fmriprep.load_confounds we may ' + 'include it in the future', ValueError + ) + if v not in ['basic', 'power2', 'derivatives', 'full']: + raise_error( + f'{v} not a valid type of confound to extract from {k} ' + 'signal!', ValueError + ) def validate_input(self, input): """Validate the input to the pipeline step. @@ -52,20 +118,26 @@ class BaseConfoundRemover(PipelineStepMixin): ValueError: If the input does not have the required data. """ - for k, v in self.strategy.items(): - if k not in ['motion','wm_csf','global_signal']: - raise ValueError( - f'{k} not a valid noise component to include!\n' - f'If {k} is a valid parameter in ' - 'nilearn.interfaces.fmriprep.load_confounds we may ' - 'include it in the future' - ) - if v not in ['basic','power2','derivatives','full']: - raise ValueError( - f'{v} not a valid type of confound to extract from input ' - 'signal!' - ) + if 'BOLD' not in input: + raise_error( + 'Input does not have the required data. \n' + f'Input: {input} \n' + f'Required: BOLD \n', ValueError + ) + if 'data' not in input['BOLD']: + raise_error( + 'Input does not have the required data. \n' + f'Input: {input} \n' + f'Required: [\'BOLD\'][\'data\'] \n', ValueError + ) + # check that keys actually return a valid 4d image + # and that confounds are a dataframe + check_niimg_4d(input['BOLD']['data']) + if not isinstance(input['confounds'], pd.DataFrame): + raise_error( + 'Input does not have the required data.', ValueError + ) def get_output_kind(self, input): """Get the kind of the pipeline step. @@ -80,29 +152,85 @@ class BaseConfoundRemover(PipelineStepMixin): output : Junifer Data dictionary The output of the pipeline step. """ - raise NotImplementedError('get_output_kind not implemented') + return 'BOLD' - def read_confounds(self): - raise NotImplementedError('read_confounds not implemented') + def read_confounds(self, confound_dataframe): + """ Select relevant confounds from the specified file """ - def remove_confounds(self): - pass + logger.warning( + 'BaseConfoundRemover removes all confounds from the file without' + ' applying any selection strategy!' + ) + return confound_dataframe + + def remove_confounds(self, bold_img, confounds_df): + """ Remove confounds from the BOLD image + + bold_img : Niimg-like object + 4D image. The signals in the last dimension are filtered + (see http://nilearn.github.io/manipulating_images/input_output.html + for a detailed description of the valid input types). + confounds_df : pd.DataFrame + Dataframe containing confounds to remove. Number of rows should + correspond to number of volumes in the BOLD image. + + returns + -------- + clean_bold : Niimg-like object + input image, cleaned. + + """ + + confounds_array = confounds_df.values + + assert bold_img.get_fdata().shape[3] == confounds_array.shape[0], ( + 'Image time series and confounds have different length!' + ) + + if self.t_r is None: + logger.warning('No t_r specified, using t_r from nifti header!') + zooms = bold_img.header.get_zooms() + self.t_r = zooms[3] + + if self.mask_img is None: + self.mask_img = compute_brain_mask(bold_img) + + clean_bold = clean_img( + imgs=bold_img, + detrend=self.detrend, + standardize=self.standardize, + confounds=confounds_array, + low_pass=self.low_pass, + high_pass=self.high_pass, + t_r=self.t_r, + mask_img=self.mask_img + ) + + return clean_bold def fit_transform(self, input): + self.validate_input(input) out = {} meta = input.get('meta', {}) bold_img = input['BOLD']['data'] - confounds_df = input['confounds'] - selected_confounds = self.read_confounds(confounds_df) - clean_img = self.remove_confounds(bold_img, confounds_df) - + confounds_df = self.read_confounds(input['confounds']) + out['BOLD'] = {} + out['BOLD']['data'] = self.remove_confounds(bold_img, confounds_df) + out['confounds'] = confounds_df + out['meta'] = meta return out + class FelixConfoundRemover(BaseConfoundRemover): + """ A class to read confounds from confound files generated by Felix's + pipeline using CAT and some other custom scripts. It is meant to emulate + the new nilearn.interfaces.fmriprep.load_confounds as closely as possible. + + """ def read_confounds(self, confound_dataframe): - + confounds_to_select = [] # for some confounds we need to manually calculate derivatives using # numpy and add them to the output @@ -110,21 +238,21 @@ class FelixConfoundRemover(BaseConfoundRemover): for comp, param in self.strategy.items(): if comp == 'motion': - - # there should be six rigid body parameters - for i in range(1,7): + confounds = [] + # there should be six rigid body parameters + for i in range(1, 7): # select basic confounds_to_select.append(f'RP.{i}') - - # select squares + + # select squares if param in ['power2', 'full']: confounds_to_select.append(f'RP^2.{i}') - + # select derivatives if param in ['derivatives', 'full']: confounds_to_select.append(f'DRP.{i}') - - # if 'full' we should not forget the derivative + + # if 'full' we should not forget the derivative # of the squares if param in ['full']: confounds_to_select.append(f'DRP^2.{i}') @@ -133,11 +261,11 @@ class FelixConfoundRemover(BaseConfoundRemover): confounds = ['WM', 'CSF'] elif comp == 'global_signal': confounds = ['GS'] - + for conf in confounds: confounds_to_select.append(conf) - + # select squares if param in ['power2', 'full']: confounds_to_select.append(f'{conf}^2') @@ -146,19 +274,28 @@ class FelixConfoundRemover(BaseConfoundRemover): # confound files) if param in ['derivatives', 'full']: derivatives_to_calculate.append(conf) - + if param in ['full']: derivatives_to_calculate.append(f'{conf}^2') confounds_to_remove = confound_dataframe[confounds_to_select] - + + # calc additional derivatives for conf in derivatives_to_calculate: confounds_to_remove[f'D{conf}'] = np.append( np.diff(confound_dataframe[conf]), 0 ) + # add binary spike regressor if needed at given threshold + if self.spike is not None: + fd = confound_dataframe["FD"].copy() + fd.loc[fd > self.spike] = 1 + fd.loc[fd != 1] = 0 + confounds_to_remove['spike'] = fd + return confounds_to_remove + class FmriprepConfoundRemover(BaseConfoundRemover): """ A ConfoundRemover class for fmriprep output utilising nilearn's nilearn.interfaces.fmriprep.load_confounds @@ -166,4 +303,3 @@ class FmriprepConfoundRemover(BaseConfoundRemover): def read_confounds(self): raise NotImplementedError('read_confounds not implemented') - \ No newline at end of file diff --git a/junifer/preprocess/tests/test_confounds.py b/junifer/preprocess/tests/test_confounds.py new file mode 100644 index 000000000..baf635e3b --- /dev/null +++ b/junifer/preprocess/tests/test_confounds.py @@ -0,0 +1,140 @@ +# Authors: Federico Raimondo +# Leonard Sasse +# License: AGPL + +import numpy as np +import pandas as pd +from nibabel import Nifti1Image +from nilearn._utils.niimg_conversions import check_niimg_4d +import random +import string +from junifer.preprocess.confounds import FelixConfoundRemover + +np.random.seed(1234567) + + +def generate_conf_name(size=6, chars=string.ascii_uppercase + string.digits): + return ''.join(random.choice(chars) for _ in range(size)) + + +def _simu_img(): + + # Random 4D volume with 100 time points + vol = 100 + 10 * np.random.randn(5, 5, 2, 100) + img = Nifti1Image(vol, np.eye(4)) + # Create an nifti image with the data, and corresponding mask + mask = Nifti1Image(np.ones([5, 5, 2]), np.eye(4)) + return img, mask + + +def test_felixconfoundremover(): + # Generate a simulated BOLD img + siimg, simsk = _simu_img() + + # generate random confound dataframe with Felix's column naming + confound_column_names = [] + + confound_column_names.append('FD') + + for i in range(1, 7): + confound_column_names.append(f'RP.{i}') + confound_column_names.append(f'RP^2.{i}') + confound_column_names.append(f'DRP.{i}') + confound_column_names.append(f'DRP^2.{i}') + + for conf in ['WM', 'CSF', 'GS']: + confound_column_names.append(conf) + confound_column_names.append(f'{conf}^2') + + # add some random irrelevant confounds + for i in range(10): + confound_column_names.append(generate_conf_name()) + + np.random.shuffle(confound_column_names) + n_cols = len(confound_column_names) + confounds_df = pd.DataFrame( + np.random.randint(0, 100, size=(100, n_cols)), + columns=confound_column_names + ) + + # generate confound removal strategies with varying numbers of parameters + list_of_strategy_tuples = [] + + # 36 params + strat1 = { + 'motion': 'full', + 'wm_csf': 'full', + 'global_signal': 'full' + } + list_of_strategy_tuples.append((strat1, 36)) + + # 24 params + strat2 = { + 'motion': 'full', + } + list_of_strategy_tuples.append((strat2, 24)) + + # 9 params + strat3 = { + 'motion': 'basic', + 'wm_csf': 'basic', + 'global_signal': 'basic' + } + list_of_strategy_tuples.append((strat3, 9)) + + # 6 params + strat4 = { + 'motion': 'basic', + } + list_of_strategy_tuples.append((strat4, 6)) + + # 2 params + strat5 = { + 'wm_csf': 'basic', + } + list_of_strategy_tuples.append((strat5, 2)) + + # generate a junifer pipeline data object dictionary + input_data_obj = {} + input_data_obj['meta'] = {} + input_data_obj['BOLD'] = {} + input_data_obj['BOLD']['data'] = siimg + input_data_obj['confounds'] = confounds_df + + # run confound removal + for spike in [None, 0.25]: + for strategy, n_params in list_of_strategy_tuples: + FCR = FelixConfoundRemover( + strategy=strategy, + spike=spike, + mask_img=simsk, + t_r=0.75 + ) + + # first test some methods manually + FCR.validate_input(input_data_obj) + out_type = FCR.get_output_kind(input_data_obj) + + assert out_type in ['BOLD'] + + selected_conf_frame = FCR.read_confounds(confounds_df) + timepoints, n_sel_confs = selected_conf_frame.shape + + assert timepoints == 100 + assert isinstance(selected_conf_frame, pd.DataFrame) + + if spike is None: + assert n_sel_confs == n_params + else: + assert n_sel_confs == n_params + 1 + + cl_bold = FCR.remove_confounds(siimg, selected_conf_frame) + check_niimg_4d(cl_bold) + + # finally test fit_transform + output_data_obj = FCR.fit_transform(input_data_obj) + + for key in ['BOLD', 'confounds', 'meta']: + assert key in output_data_obj + + check_niimg_4d(output_data_obj['BOLD']['data']) -- 2.52.0 From d799c7028696a01b5646cf8057982981ff6db25f Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 31 Mar 2022 15:36:52 +0200 Subject: [PATCH 32/40] In theory, this should work --- junifer/markers/base.py | 15 +- junifer/preprocess/confounds.py | 465 ++++++++++++++++++++------------ 2 files changed, 299 insertions(+), 181 deletions(-) diff --git a/junifer/markers/base.py b/junifer/markers/base.py index 9d736aa15..0471744d0 100644 --- a/junifer/markers/base.py +++ b/junifer/markers/base.py @@ -18,8 +18,9 @@ class PipelineStepMixin(): Parameters ---------- - input : Junifer Data dictionary - The input to the pipeline step. + input : list[str] + The input to the pipeline step. The list must contain the + available Junifer Data dictionary keys. Raises ------ @@ -33,13 +34,15 @@ class PipelineStepMixin(): Parameters ---------- - input : Junifer Data dictionary - The input to the pipeline step. + input : list[str] + The input to the pipeline step. The list must contain the + available Junifer Data dictionary keys. Returns ------- - output : Junifer Data dictionary - The output of the pipeline step. + output : list[str] + The updated list of available Junifer Data dictionary keys after + the pipeline step. """ raise NotImplementedError('get_output_kind not implemented') diff --git a/junifer/preprocess/confounds.py b/junifer/preprocess/confounds.py index 774dc36a1..632bdbd24 100644 --- a/junifer/preprocess/confounds.py +++ b/junifer/preprocess/confounds.py @@ -25,62 +25,56 @@ class BaseConfoundRemover(PipelineStepMixin): # in particular scrubbing # TODO: Implement read_confounds for fmriprep data - def __init__( - self, - strategy={ - 'motion': 'full', - 'wm_csf': 'full', - 'global_signal': 'full' - }, - spike=None, - detrend=True, - standardize=True, - low_pass=None, - high_pass=None, - t_r=None, - mask_img=None - ): + def __init__(self, strategy=None, spike=None, detrend=True, + standardize=True, low_pass=None, high_pass=None, t_r=None, + mask_img=None): """ Initialise a BaseConfoundReader object Confound removal is based on nilearn.image.clean_img Parameters ----------- - strategy : dict - keys of dictionary should correspond to names of noise components - to include: + strategy : dict[str -> str] + The keys of the dictionary should correspond to names of noise + components to include: - 'motion' - 'wm_csf' - 'global_signal' - values of dictionary should correspond to types of confounds + The values of dictionary should correspond to types of confounds extracted from each signal: - - 'basic' - only the confounding time series - - 'power2' - signal + quadratic term - - 'derivatives' - signal + derivatives - - 'full' - signal + deriv. + quadratic terms + power2 deriv. - spike : None (default) | float + - 'basic': only the confounding time series + - 'power2': signal + quadratic term + - 'derivatives': signal + derivatives + - 'full': signal + deriv. + quadratic terms + power2 deriv. + spike : float | None (default) If None, no spike regressor is added. If spike is a float, it will add a spike regressor for every point at which FD exceeds the specified float. detrend : bool - If detrending should be applied on timeseries - (before confound removal). Default=True. + If True (default), detrending will be applied on timeseries + (before confound removal). standardize : bool - If True, returned signals are set to unit variance. Default=True. - low_pass : float - Low cutoff frequencies, in Hertz. - high_pass : float - High cutoff frequencies, in Hertz. + If True (default), returned signals are set to unit variance. + low_pass : float | None (default) + Low cutoff frequencies, in Hertz. If None, no filtering is applied. + high_pass : float | None (default) + High cutoff frequencies, in Hertz. If None, no filtering is + applied. t_r : float Repetition time, in second (sampling period). - If set to None it will use t_r from nifti header. + If None (default) it will use t_r from nifti header. mask_img: Niimg-like object If provided, signal is only cleaned from voxels inside the mask. If mask is provided, it should have same shape and affine as imgs. If not provided, a mask is computed using nilearn.masking.compute_brain_mask - """ + if strategy is None: + strategy = { + 'motion': 'full', + 'wm_csf': 'full', + 'global_signal': 'full' + } self.strategy = strategy self.spike = spike @@ -91,52 +85,57 @@ class BaseConfoundRemover(PipelineStepMixin): self.t_r = t_r self.mask_img = mask_img - for k, v in self.strategy.items(): - if k not in ['motion', 'wm_csf', 'global_signal']: - raise_error( - f'{k} not a valid noise component to include!\n' - f'If {k} is a valid parameter in ' - 'nilearn.interfaces.fmriprep.load_confounds we may ' - 'include it in the future', ValueError - ) - if v not in ['basic', 'power2', 'derivatives', 'full']: - raise_error( - f'{v} not a valid type of confound to extract from {k} ' - 'signal!', ValueError - ) + self._valid_components = ['motion', 'wm_csf', 'global_signal'] + self._valid_confounds = ['basic', 'power2', 'derivatives', 'full'] + + if any(not isinstance(k, str) for k in strategy.keys()): + raise_error( + 'Strategy keys must be strings', ValueError + ) + + if any(not isinstance(v, str) for v in strategy.values()): + raise_error( + 'Strategy values must be strings', ValueError + ) + + if any(x not in self._valid_components for x in strategy.keys()): + raise_error( + f'Invalid component names {list(strategy.keys())}. ' + f'Valid components are {self._valid_components}.\n' + f'If any of them is a valid parameter in ' + 'nilearn.interfaces.fmriprep.load_confounds we may ' + 'include it in the future', ValueError + ) + + if any(x not in self._valid_confounds for x in strategy.values()): + raise_error( + f'Invalid component names {list(strategy.values())}. ' + f'Valid confound types are {self._valid_confounds}.\n' + f'If any of them is a valid parameter in ' + 'nilearn.interfaces.fmriprep.load_confounds we may ' + 'include it in the future', ValueError + ) def validate_input(self, input): """Validate the input to the pipeline step. Parameters ---------- - input : Junifer Data dictionary - The input to the pipeline step. + input : list[str] + The input to the pipeline step. The list must contain the + available Junifer Data dictionary keys. Raises ------ ValueError: If the input does not have the required data. """ - if 'BOLD' not in input: + _required_inputs = ['BOLD', 'confounds'] + if any(x not in input for x in _required_inputs): raise_error( 'Input does not have the required data. \n' f'Input: {input} \n' - f'Required: BOLD \n', ValueError - ) - if 'data' not in input['BOLD']: - raise_error( - 'Input does not have the required data. \n' - f'Input: {input} \n' - f'Required: [\'BOLD\'][\'data\'] \n', ValueError - ) - - # check that keys actually return a valid 4d image - # and that confounds are a dataframe - check_niimg_4d(input['BOLD']['data']) - if not isinstance(input['confounds'], pd.DataFrame): - raise_error( - 'Input does not have the required data.', ValueError + f'Required (all off): {_required_inputs} \n', ValueError ) def get_output_kind(self, input): @@ -144,26 +143,50 @@ class BaseConfoundRemover(PipelineStepMixin): Parameters ---------- - input : Junifer Data dictionary - The input to the pipeline step. + input : list[str] + The input to the pipeline step. The list must contain the + available Junifer Data dictionary keys. Returns ------- - output : Junifer Data dictionary - The output of the pipeline step. + output : list[str] + The updated list of available Junifer Data dictionary keys after + the pipeline step. """ - return 'BOLD' + # Does not add any new keys + return input - def read_confounds(self, confound_dataframe): + def _pick_confounds(self, input): """ Select relevant confounds from the specified file """ + to_select = [] + confounds_df = input['data'] + confounds_spec = input['names']['spec'] + derivatives_to_compute = input['names']['derivatives'] + spike_name = input['names']['spike'] - logger.warning( - 'BaseConfoundRemover removes all confounds from the file without' - ' applying any selection strategy!' - ) - return confound_dataframe + # Get all the column names according to the strategy + for comp, param in self.strategy.items(): + to_select.append(confounds_spec[comp][param]) - def remove_confounds(self, bold_img, confounds_df): + # Add derivatives if needed + to_compute = [x in derivatives_to_compute.keys() for x in to_select] + out_df = confounds_df.copy() + if any(to_compute): + for t_dst in to_compute: + t_src = derivatives_to_compute[t_dst] + out_df[t_dst] = np.append( # type: ignore + np.diff(out_df[t_src]), 0) # type: ignore + + # add binary spike regressor if needed at given threshold + if self.spike is not None: + fd = confounds_df[spike_name].copy() + fd.loc[fd > self.spike] = 1 + fd.loc[fd != 1] = 0 + out_df['spike'] = fd + + return out_df + + def _remove_confounds(self, bold_img, confounds_df): """ Remove confounds from the BOLD image bold_img : Niimg-like object @@ -183,17 +206,17 @@ class BaseConfoundRemover(PipelineStepMixin): confounds_array = confounds_df.values - assert bold_img.get_fdata().shape[3] == confounds_array.shape[0], ( - 'Image time series and confounds have different length!' - ) - - if self.t_r is None: - logger.warning('No t_r specified, using t_r from nifti header!') + t_r = self.t_r + if t_r is None: + logger.info('No t_r specified, using t_r from nifti header!') zooms = bold_img.header.get_zooms() - self.t_r = zooms[3] + t_r = zooms[3] + logger.info(f'Read t_r from nifti header: {t_r}', ) - if self.mask_img is None: - self.mask_img = compute_brain_mask(bold_img) + mask_img = self.mask_img + if mask_img is None: + logger.info('Computing brain mask from image') + mask_img = compute_brain_mask(bold_img) clean_bold = clean_img( imgs=bold_img, @@ -202,104 +225,196 @@ class BaseConfoundRemover(PipelineStepMixin): confounds=confounds_array, low_pass=self.low_pass, high_pass=self.high_pass, - t_r=self.t_r, - mask_img=self.mask_img + t_r=t_r, + mask_img=mask_img ) return clean_bold - def fit_transform(self, input): - self.validate_input(input) - out = {} - meta = input.get('meta', {}) - bold_img = input['BOLD']['data'] - confounds_df = self.read_confounds(input['confounds']) - out['BOLD'] = {} - out['BOLD']['data'] = self.remove_confounds(bold_img, confounds_df) - out['confounds'] = confounds_df - out['meta'] = meta + def _validate_data(self, input): + # Bold must be 4D niimg + check_niimg_4d(input['BOLD']['data']) - return out - - -class FelixConfoundRemover(BaseConfoundRemover): - """ A class to read confounds from confound files generated by Felix's - pipeline using CAT and some other custom scripts. It is meant to emulate - the new nilearn.interfaces.fmriprep.load_confounds as closely as possible. - - """ - - def read_confounds(self, confound_dataframe): - - confounds_to_select = [] - # for some confounds we need to manually calculate derivatives using - # numpy and add them to the output - derivatives_to_calculate = [] - - for comp, param in self.strategy.items(): - if comp == 'motion': - confounds = [] - # there should be six rigid body parameters - for i in range(1, 7): - # select basic - confounds_to_select.append(f'RP.{i}') - - # select squares - if param in ['power2', 'full']: - confounds_to_select.append(f'RP^2.{i}') - - # select derivatives - if param in ['derivatives', 'full']: - confounds_to_select.append(f'DRP.{i}') - - # if 'full' we should not forget the derivative - # of the squares - if param in ['full']: - confounds_to_select.append(f'DRP^2.{i}') - - elif comp == 'wm_csf': - confounds = ['WM', 'CSF'] - elif comp == 'global_signal': - confounds = ['GS'] - - for conf in confounds: - - confounds_to_select.append(conf) - - # select squares - if param in ['power2', 'full']: - confounds_to_select.append(f'{conf}^2') - - # we have to calculate derivatives (not included in felix' - # confound files) - if param in ['derivatives', 'full']: - derivatives_to_calculate.append(conf) - - if param in ['full']: - derivatives_to_calculate.append(f'{conf}^2') - - confounds_to_remove = confound_dataframe[confounds_to_select] - - # calc additional derivatives - for conf in derivatives_to_calculate: - confounds_to_remove[f'D{conf}'] = np.append( - np.diff(confound_dataframe[conf]), 0 + # Confounds must be a dataframe + if not isinstance(input['confounds']['data'], pd.DataFrame): + raise_error( + 'confounds data must be a pandas dataframe', ValueError ) - # add binary spike regressor if needed at given threshold - if self.spike is not None: - fd = confound_dataframe["FD"].copy() - fd.loc[fd > self.spike] = 1 - fd.loc[fd != 1] = 0 - confounds_to_remove['spike'] = fd + confound_df = input['confounds']['data'] + bold_img = input['BOLD']['data'] + if bold_img.get_fdata().shape[3] != len(confound_df): + raise_error( + 'Image time series and confounds have different length!\n' + f'\tImage time series: { bold_img.get_fdata().shape[3]}\n' + f'\tConfounds: {len(confound_df)}') - return confounds_to_remove + # Check the column names of the dataframe and the spec + # spec must be a dictionary: + # { + # 'motion': { + # 'basic': [(list of columns)] + # 'power2', [(list of columns)] + # 'derivatives', [(list of columns)] + # 'full', [(list of columns)]}, + # 'wm_csf': { + # 'basic': [(list of columns)] + # 'power2', [(list of columns)] + # 'derivatives', [(list of columns)] + # 'full', [(list of columns)]} + # 'global_signal': { + # 'basic': [(list of columns)] + # 'power2', [(list of columns)] + # 'derivatives', [(list of columns)] + # 'full', [(list of columns)]} + # } + + # Check the columns in the dataframe + conf_spec = input['confounds']['names']['spec'] + if any(x not in conf_spec.keys() for x in self._valid_components): + raise_error( + 'All of the component types must be in the confounds data ' + 'object `spec`. Please check your datagrabber.', ValueError) + + if any(x not in v.keys() for x in self._valid_confounds + for v in conf_spec.values()): + raise_error( + 'All of the confound types must be in the confounds data ' + 'object `spec`. Please check your datagrabber.', ValueError) + + spike_name = input['confounds']['names']['spike'] + + derivatives_to_compute = input['confounds']['names']['derivatives'] + if not(isinstance(derivatives_to_compute, dict)): + raise_error( + 'input["confounds"]["names"]["derivatives"] ' + 'must be a dictionary. Please check your datagrabber', + ValueError) + + if any(not (isinstance(k, str) or isinstance(v, str)) + for k, v in derivatives_to_compute.items()): + raise_error( + 'input["confounds"]["names"]["derivatives"] ' + 'must be a dictionary with string keys and values. ' + 'Please check your datagrabber', + ValueError) + + missing_derivatives = [ + x for x in derivatives_to_compute.values() + if x not in confound_df.columns] + if len(missing_derivatives) > 0: + raise_error( + 'Some of the derivatives to calculate are not in the confounds' + f' dataframe: {missing_derivatives}.' + 'Please check your data ' + f'({input["confounds"]["path"].as_posix()}) ' + 'and the datagrabber.', ValueError) + + column_names = set([x for y in conf_spec.values() for x in y]) + column_names.add(spike_name) + + missing_columns = [ + x for x in column_names + if x not in confound_df.columns and + x not in derivatives_to_compute.keys()] + + if len(missing_columns) > 0: + raise_error( + 'Some of the columns in the confound spec are not in the ' + f'confounds dataframe: {missing_columns}. ' + 'Please check your data ' + f'({input["confounds"]["path"].as_posix()}) ' + 'and the datagrabber.', ValueError) + + def fit_transform(self, input): + self._validate_data(input) + bold_img = input['BOLD']['data'] + confounds_df = self._pick_confounds(input['confounds']) + input['BOLD']['data'] = self._remove_confounds(bold_img, confounds_df) + + # TODO: Update meta + return input -class FmriprepConfoundRemover(BaseConfoundRemover): - """ A ConfoundRemover class for fmriprep output utilising - nilearn's nilearn.interfaces.fmriprep.load_confounds - """ +# class FelixConfoundRemover(BaseConfoundRemover): +# """ A class to read confounds from confound files generated by Felix's +# pipeline using CAT and some other custom scripts. It is meant to emulate +# the new nilearn.interfaces.fmriprep.load_confounds as closely as +# possible. - def read_confounds(self): - raise NotImplementedError('read_confounds not implemented') +# """ + +# def read_confounds(self, confound_dataframe): + +# confounds_to_select = [] +# # for some confounds we need to manually calculate derivatives using +# # numpy and add them to the output +# derivatives_to_compute = [] + +# for comp, param in self.strategy.items(): +# if comp == 'motion': +# confounds = [] +# # there should be six rigid body parameters +# for i in range(1, 7): +# # select basic +# confounds_to_select.append(f'RP.{i}') + +# # select squares +# if param in ['power2', 'full']: +# confounds_to_select.append(f'RP^2.{i}') + +# # select derivatives +# if param in ['derivatives', 'full']: +# confounds_to_select.append(f'DRP.{i}') + +# # if 'full' we should not forget the derivative +# # of the squares +# if param in ['full']: +# confounds_to_select.append(f'DRP^2.{i}') + +# elif comp == 'wm_csf': +# confounds = ['WM', 'CSF'] +# elif comp == 'global_signal': +# confounds = ['GS'] + +# for conf in confounds: + +# confounds_to_select.append(conf) + +# # select squares +# if param in ['power2', 'full']: +# confounds_to_select.append(f'{conf}^2') + +# # we have to calculate derivatives (not included in felix' +# # confound files) +# if param in ['derivatives', 'full']: +# derivatives_to_compute.append(conf) + +# if param in ['full']: +# derivatives_to_compute.append(f'{conf}^2') + +# confounds_to_remove = confound_dataframe[confounds_to_select] + +# # calc additional derivatives +# for conf in derivatives_to_compute: +# confounds_to_remove[f'D{conf}'] = np.append( +# np.diff(confound_dataframe[conf]), 0 +# ) + +# # add binary spike regressor if needed at given threshold +# if self.spike is not None: +# fd = confound_dataframe["FD"].copy() +# fd.loc[fd > self.spike] = 1 +# fd.loc[fd != 1] = 0 +# confounds_to_remove['spike'] = fd + +# return confounds_to_remove + + +# class FmriprepConfoundRemover(BaseConfoundRemover): +# """ A ConfoundRemover class for fmriprep output utilising +# nilearn's nilearn.interfaces.fmriprep.load_confounds +# """ + +# def read_confounds(self): +# raise NotImplementedError('read_confounds not implemented') -- 2.52.0 From 98e344145ea6410924fd25dfb264ba6d35a5455a Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Tue, 12 Apr 2022 13:10:48 +0300 Subject: [PATCH 33/40] WIP: Multiple datagrabber + test confound removal --- junifer/datagrabber/meta.py | 32 ++ junifer/preprocess/confounds.py | 21 +- junifer/preprocess/tests/test_confounds.py | 350 ++++++++++++++++----- 3 files changed, 319 insertions(+), 84 deletions(-) create mode 100644 junifer/datagrabber/meta.py diff --git a/junifer/datagrabber/meta.py b/junifer/datagrabber/meta.py new file mode 100644 index 000000000..1b20949d2 --- /dev/null +++ b/junifer/datagrabber/meta.py @@ -0,0 +1,32 @@ +from .base import BaseDataGrabber + + +class MultipleDataGrabber(BaseDataGrabber): + def __init__(self, datagrabbers): + # TODO: Check datagrabbers consistency + # - same element keys + # - no overlapping types + self._datagrabbers = datagrabbers + + def get_types(self): + types = [x for dg in self._datagrabbers for x in dg.get_types()] + return types + + def get_meta(self): + t_meta = {} + t_meta['class'] = self.__class__.__name__ + t_meta['datagrabbers'] = [dg.get_meta() for dg in self._datagrabbers] + + def __enter__(self): + for dg in self._datagrabbers: + dg.__enter__() + + def __exit__(self, exc_type, exc_value, exc_traceback): + for dg in self._datagrabbers: + dg.__exit__(exc_type, exc_value, exc_traceback) + + def __getitem__(self, element): + out = {} + for dg in self._datagrabbers: + t_out = dg[element] + out.update(t_out) diff --git a/junifer/preprocess/confounds.py b/junifer/preprocess/confounds.py index 632bdbd24..0d56f818a 100644 --- a/junifer/preprocess/confounds.py +++ b/junifer/preprocess/confounds.py @@ -161,22 +161,21 @@ class BaseConfoundRemover(PipelineStepMixin): to_select = [] confounds_df = input['data'] confounds_spec = input['names']['spec'] - derivatives_to_compute = input['names']['derivatives'] + derivatives_to_compute = input['names'].get('derivatives', {}) spike_name = input['names']['spike'] # Get all the column names according to the strategy for comp, param in self.strategy.items(): - to_select.append(confounds_spec[comp][param]) + to_select.extend(confounds_spec[comp][param]) # Add derivatives if needed to_compute = [x in derivatives_to_compute.keys() for x in to_select] out_df = confounds_df.copy() if any(to_compute): - for t_dst in to_compute: - t_src = derivatives_to_compute[t_dst] - out_df[t_dst] = np.append( # type: ignore - np.diff(out_df[t_src]), 0) # type: ignore - + for t_dst, t_src in derivatives_to_compute.items(): + out_df[t_dst] = np.append( # type: ignore + np.diff(out_df[t_src]), 0) # type: ignore + out_df = out_df[to_select] # add binary spike regressor if needed at given threshold if self.spike is not None: fd = confounds_df[spike_name].copy() @@ -284,7 +283,8 @@ class BaseConfoundRemover(PipelineStepMixin): spike_name = input['confounds']['names']['spike'] - derivatives_to_compute = input['confounds']['names']['derivatives'] + derivatives_to_compute = input['confounds']['names'].get( + 'derivatives', {}) if not(isinstance(derivatives_to_compute, dict)): raise_error( 'input["confounds"]["names"]["derivatives"] ' @@ -310,7 +310,10 @@ class BaseConfoundRemover(PipelineStepMixin): f'({input["confounds"]["path"].as_posix()}) ' 'and the datagrabber.', ValueError) - column_names = set([x for y in conf_spec.values() for x in y]) + t_conf_spec = {k: input['confounds']['names']['spec'][k][v] + for k, v in self.strategy.items()} + + column_names = set([x for y in t_conf_spec.values() for x in y]) column_names.add(spike_name) missing_columns = [ diff --git a/junifer/preprocess/tests/test_confounds.py b/junifer/preprocess/tests/test_confounds.py index baf635e3b..ede77c861 100644 --- a/junifer/preprocess/tests/test_confounds.py +++ b/junifer/preprocess/tests/test_confounds.py @@ -2,13 +2,14 @@ # Leonard Sasse # License: AGPL +from pathlib import Path import numpy as np import pandas as pd from nibabel import Nifti1Image from nilearn._utils.niimg_conversions import check_niimg_4d import random import string -from junifer.preprocess.confounds import FelixConfoundRemover +from junifer.preprocess.confounds import BaseConfoundRemover np.random.seed(1234567) @@ -27,24 +28,45 @@ def _simu_img(): return img, mask -def test_felixconfoundremover(): +def test_baseconfoundremover(): # Generate a simulated BOLD img siimg, simsk = _simu_img() # generate random confound dataframe with Felix's column naming + + motion_basic = [f'RP.{i}' for i in range(1, 7)] + motion_power2 = [f'RP^2.{i}' for i in range(1, 7)] + motion_derivatives = [f'DRP.{i}' for i in range(1, 7)] + motion_full = [f'DRP^2.{i}' for i in range(1, 7)] + + wm_csf_basic = ['WM', 'CSF'] + wm_csf_power2 = ['WM^2', 'CSF^2'] + wm_csf_derivatives = ['DWM', 'DCSF'] + wm_csf_full = ['DWM^2', 'DCSF^2'] + + gs_basic = ['GS'] + gs_power2 = ['GS^2'] + gs_derivatives = ['DGS'] + gs_full = ['DGS^2'] + confound_column_names = [] - confound_column_names.append('FD') + confound_column_names.append('FD') # spike - for i in range(1, 7): - confound_column_names.append(f'RP.{i}') - confound_column_names.append(f'RP^2.{i}') - confound_column_names.append(f'DRP.{i}') - confound_column_names.append(f'DRP^2.{i}') + confound_column_names.extend(motion_basic) + confound_column_names.extend(motion_power2) + confound_column_names.extend(motion_derivatives) + confound_column_names.extend(motion_full) - for conf in ['WM', 'CSF', 'GS']: - confound_column_names.append(conf) - confound_column_names.append(f'{conf}^2') + confound_column_names.extend(wm_csf_basic) + confound_column_names.extend(wm_csf_power2) + confound_column_names.extend(wm_csf_derivatives) + confound_column_names.extend(wm_csf_full) + + confound_column_names.extend(gs_basic) + confound_column_names.extend(gs_power2) + confound_column_names.extend(gs_derivatives) + confound_column_names.extend(gs_full) # add some random irrelevant confounds for i in range(10): @@ -57,84 +79,262 @@ def test_felixconfoundremover(): columns=confound_column_names ) - # generate confound removal strategies with varying numbers of parameters - list_of_strategy_tuples = [] - - # 36 params - strat1 = { - 'motion': 'full', - 'wm_csf': 'full', - 'global_signal': 'full' + # Generate spec from Felix's column naming + spec = { + 'motion': { + 'basic': motion_basic, + 'power2': motion_basic + motion_power2, + 'derivatives': motion_basic + motion_derivatives, + 'full': + motion_basic + motion_derivatives + motion_power2 + motion_full + }, + 'wm_csf': { + 'basic': wm_csf_basic, + 'power2': wm_csf_basic + wm_csf_power2, + 'derivatives': wm_csf_basic + wm_csf_derivatives, + 'full': + wm_csf_basic + wm_csf_derivatives + wm_csf_power2 + wm_csf_full + }, + 'global_signal': { + 'basic': gs_basic, + 'power2': gs_basic + gs_power2, + 'derivatives': gs_basic + gs_derivatives, + 'full': gs_basic + gs_derivatives + gs_power2 + gs_full + } } - list_of_strategy_tuples.append((strat1, 36)) - - # 24 params - strat2 = { - 'motion': 'full', - } - list_of_strategy_tuples.append((strat2, 24)) - - # 9 params - strat3 = { - 'motion': 'basic', - 'wm_csf': 'basic', - 'global_signal': 'basic' - } - list_of_strategy_tuples.append((strat3, 9)) - - # 6 params - strat4 = { - 'motion': 'basic', - } - list_of_strategy_tuples.append((strat4, 6)) - - # 2 params - strat5 = { - 'wm_csf': 'basic', - } - list_of_strategy_tuples.append((strat5, 2)) # generate a junifer pipeline data object dictionary input_data_obj = {} input_data_obj['meta'] = {} input_data_obj['BOLD'] = {} input_data_obj['BOLD']['data'] = siimg - input_data_obj['confounds'] = confounds_df + input_data_obj['confounds'] = {} + input_data_obj['confounds']['path'] = Path('/test.df') + input_data_obj['confounds']['data'] = confounds_df + input_data_obj['confounds']['names'] = {} + input_data_obj['confounds']['names']['spec'] = spec + input_data_obj['confounds']['names']['spike'] = 'FD' - # run confound removal - for spike in [None, 0.25]: - for strategy, n_params in list_of_strategy_tuples: - FCR = FelixConfoundRemover( - strategy=strategy, - spike=spike, - mask_img=simsk, - t_r=0.75 - ) + # generate confound removal strategies with varying numbers of parameters - # first test some methods manually - FCR.validate_input(input_data_obj) - out_type = FCR.get_output_kind(input_data_obj) + # Test #1: 36 params, no derivatives to compute, no spike + # 36 params + strat1 = { + 'motion': 'full', + 'wm_csf': 'full', + 'global_signal': 'full' + } - assert out_type in ['BOLD'] + cr = BaseConfoundRemover( + strategy=strat1, spike=None, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) - selected_conf_frame = FCR.read_confounds(confounds_df) - timepoints, n_sel_confs = selected_conf_frame.shape + assert 'BOLD' in out_type - assert timepoints == 100 - assert isinstance(selected_conf_frame, pd.DataFrame) + # Check if the input data is valid + cr._validate_data(input_data_obj) - if spike is None: - assert n_sel_confs == n_params - else: - assert n_sel_confs == n_params + 1 + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 36 + assert all(x in t_df.columns for x in motion_basic) + assert all(x in t_df.columns for x in motion_power2) + assert all(x in t_df.columns for x in motion_derivatives) + assert all(x in t_df.columns for x in motion_full) + assert all(x in t_df.columns for x in wm_csf_basic) + assert all(x in t_df.columns for x in wm_csf_power2) + assert all(x in t_df.columns for x in wm_csf_derivatives) + assert all(x in t_df.columns for x in wm_csf_full) + assert all(x in t_df.columns for x in gs_basic) + assert all(x in t_df.columns for x in gs_power2) + assert all(x in t_df.columns for x in gs_derivatives) + assert all(x in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' not in t_df.columns - cl_bold = FCR.remove_confounds(siimg, selected_conf_frame) - check_niimg_4d(cl_bold) + # Test #2: 24 params, no derivatives to compute, no spike + # 24 params + strat2 = { + 'motion': 'full', + } - # finally test fit_transform - output_data_obj = FCR.fit_transform(input_data_obj) + cr = BaseConfoundRemover( + strategy=strat2, spike=None, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) - for key in ['BOLD', 'confounds', 'meta']: - assert key in output_data_obj + assert 'BOLD' in out_type - check_niimg_4d(output_data_obj['BOLD']['data']) + # Check if the input data is valid + cr._validate_data(input_data_obj) + + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 24 + assert all(x in t_df.columns for x in motion_basic) + assert all(x in t_df.columns for x in motion_power2) + assert all(x in t_df.columns for x in motion_derivatives) + assert all(x in t_df.columns for x in motion_full) + assert all(x not in t_df.columns for x in wm_csf_basic) + assert all(x not in t_df.columns for x in wm_csf_power2) + assert all(x not in t_df.columns for x in wm_csf_derivatives) + assert all(x not in t_df.columns for x in wm_csf_full) + assert all(x not in t_df.columns for x in gs_basic) + assert all(x not in t_df.columns for x in gs_power2) + assert all(x not in t_df.columns for x in gs_derivatives) + assert all(x not in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' not in t_df.columns + + # Test #3: 9 params, no derivatives to compute, no spike + strat3 = { + 'motion': 'basic', + 'wm_csf': 'basic', + 'global_signal': 'basic' + } + + cr = BaseConfoundRemover( + strategy=strat3, spike=None, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) + + assert 'BOLD' in out_type + + # Check if the input data is valid + cr._validate_data(input_data_obj) + + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 9 + assert all(x in t_df.columns for x in motion_basic) + assert all(x not in t_df.columns for x in motion_power2) + assert all(x not in t_df.columns for x in motion_derivatives) + assert all(x not in t_df.columns for x in motion_full) + assert all(x in t_df.columns for x in wm_csf_basic) + assert all(x not in t_df.columns for x in wm_csf_power2) + assert all(x not in t_df.columns for x in wm_csf_derivatives) + assert all(x not in t_df.columns for x in wm_csf_full) + assert all(x in t_df.columns for x in gs_basic) + assert all(x not in t_df.columns for x in gs_power2) + assert all(x not in t_df.columns for x in gs_derivatives) + assert all(x not in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' not in t_df.columns + + # Test #4: 6 params, no derivatives to compute, no spike + strat4 = { + 'motion': 'basic', + } + cr = BaseConfoundRemover( + strategy=strat4, spike=None, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) + + assert 'BOLD' in out_type + + # Check if the input data is valid + cr._validate_data(input_data_obj) + + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 6 + assert all(x in t_df.columns for x in motion_basic) + assert all(x not in t_df.columns for x in motion_power2) + assert all(x not in t_df.columns for x in motion_derivatives) + assert all(x not in t_df.columns for x in motion_full) + assert all(x not in t_df.columns for x in wm_csf_basic) + assert all(x not in t_df.columns for x in wm_csf_power2) + assert all(x not in t_df.columns for x in wm_csf_derivatives) + assert all(x not in t_df.columns for x in wm_csf_full) + assert all(x not in t_df.columns for x in gs_basic) + assert all(x not in t_df.columns for x in gs_power2) + assert all(x not in t_df.columns for x in gs_derivatives) + assert all(x not in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' not in t_df.columns + + # Test #5: 2 params, no derivatives to compute, no spike + strat5 = { + 'wm_csf': 'basic', + } + cr = BaseConfoundRemover( + strategy=strat5, spike=None, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) + + assert 'BOLD' in out_type + + # Check if the input data is valid + cr._validate_data(input_data_obj) + + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 2 + assert all(x not in t_df.columns for x in motion_basic) + assert all(x not in t_df.columns for x in motion_power2) + assert all(x not in t_df.columns for x in motion_derivatives) + assert all(x not in t_df.columns for x in motion_full) + assert all(x in t_df.columns for x in wm_csf_basic) + assert all(x not in t_df.columns for x in wm_csf_power2) + assert all(x not in t_df.columns for x in wm_csf_derivatives) + assert all(x not in t_df.columns for x in wm_csf_full) + assert all(x not in t_df.columns for x in gs_basic) + assert all(x not in t_df.columns for x in gs_power2) + assert all(x not in t_df.columns for x in gs_derivatives) + assert all(x not in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' not in t_df.columns + + out = cr.fit_transform(input_data_obj) + check_niimg_4d(out['BOLD']['data']) + # TODO: check meta + + # Test #6: 12 params, derivatives to compute, spike + to_select = [x for x in confounds_df.columns + if x not in motion_derivatives] + no_d_df = confounds_df[to_select] + input_data_obj['confounds']['data'] = no_d_df + + derivatives = { + f'D{x}': x for x in motion_basic + } + + input_data_obj['confounds']['names']['derivatives'] = derivatives + + strat6 = { + 'motion': 'derivatives', + } + cr = BaseConfoundRemover( + strategy=strat6, spike=0.75, mask_img=simsk, t_r=0.75 + ) + cr.validate_input(input_data_obj.keys()) + out_type = cr.get_output_kind(input_data_obj.keys()) + + assert 'BOLD' in out_type + + # Check if the input data is valid + cr._validate_data(input_data_obj) + + # Check that the confounds are picked correctly: + t_df = cr._pick_confounds(input_data_obj['confounds']) + assert len(t_df.columns) == 13 + assert all(x in t_df.columns for x in motion_basic) + assert all(x not in t_df.columns for x in motion_power2) + assert all(x in t_df.columns for x in motion_derivatives) + assert all(x not in t_df.columns for x in motion_full) + assert all(x not in t_df.columns for x in wm_csf_basic) + assert all(x not in t_df.columns for x in wm_csf_power2) + assert all(x not in t_df.columns for x in wm_csf_derivatives) + assert all(x not in t_df.columns for x in wm_csf_full) + assert all(x not in t_df.columns for x in gs_basic) + assert all(x not in t_df.columns for x in gs_power2) + assert all(x not in t_df.columns for x in gs_derivatives) + assert all(x not in t_df.columns for x in gs_full) + assert 'FD' not in t_df.columns + assert 'spike' in t_df.columns -- 2.52.0 From ffa75f0aba3c2d108edd2764394856221581100f Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 13 Apr 2022 13:03:45 +0300 Subject: [PATCH 34/40] Updated pattern datagrabber --- junifer/configs/juseless.py | 158 +----------------- junifer/configs/tests/test_juseless.py | 2 +- junifer/datagrabber/base.py | 72 +++++++- .../tests/test_base_datagrabber.py | 68 ++++++-- tools/create_bids_example_dataset_sessions.py | 41 +++++ 5 files changed, 166 insertions(+), 175 deletions(-) create mode 100644 tools/create_bids_example_dataset_sessions.py diff --git a/junifer/configs/juseless.py b/junifer/configs/juseless.py index 1918c5d82..be3b427f5 100644 --- a/junifer/configs/juseless.py +++ b/junifer/configs/juseless.py @@ -1,6 +1,5 @@ from ..datagrabber import PatternDataladDataGrabber from ..api.decorators import register_datagrabber -from ..utils import logger @register_datagrabber @@ -26,163 +25,8 @@ class JuselessDataladUKBVBM(PatternDataladDataGrabber): types = ['VBM_GM'] replacements = ['subject', 'session'] patterns = { - 'VBM_GM': 'm0wp1{subject}_{session}_T1w.nii.gz' + 'VBM_GM': 'm0wp1_sub-{subject}_ses-{session}_T1w.nii.gz' } super().__init__( types=types, datadir=datadir, uri=uri, rootdir=rootdir, replacements=replacements, patterns=patterns) - - def get_elements(self): - """Get the list of subjects in the dataset. - - Returns - ------- - elements : list[str] - The list of subjects in the dataset. - """ - logger.debug('Getting the list of subjects in the dataset') - elems = [] - for x in self.datadir.glob('*._T1w.nii.gz'): - sub, ses = x.name.split('_') - sub = sub.replace('m0wp1', '') - ses = ses[:5] - elems.append((sub, ses)) - return elems - - -# @register_datagrabber -# class HCP1200(PatternDataGrabber): -# """ Human Connectome Project Datalad DataGrabber class - -# Implements a DataGrabber to access the Human Connectome Project - -# """ - -# def __init__( -# self, datadir=None, subjects=None, tasks=None, phase_encodings=None -# ): -# """Initialize a HCP object. - -# Parameters -# ---------- -# datadir : str or Path -# That directory where the datalad dataset will be cloned. If None, -# (default), the datalad dataset will be cloned into a temporary -# directory. -# subjects : str or list of strings -# HCP subject ID's. If 'None' (default), all available subjects are -# selected -# tasks : str or list of strings -# HCP task sessions. If 'None' (default), all available task -# sessions are selected. Can be 'REST1', 'REST2', 'SOCIAL', 'WM', -# 'RELATIONAL', 'EMOTION', 'LANGUAGE', 'GAMBLING', 'MOTOR', or a -# list consisting of these names. -# phase_encoding : str or list of strings -# HCP phase encoding directions. Can be 'LR' or 'RL'. If 'None' -# (default) both will be used. - -# """ -# uri = ( -# 'https://github.com/datalad-datasets/' -# 'human-connectome-project-openaccess.git' -# ) -# rootdir = 'HCP1200' -# types = ['BOLD'] -# super().__init__( -# types=types, datadir=datadir, uri=uri, rootdir=rootdir -# ) - -# self.subjects = subjects -# self.tasks = tasks -# self.phase_encodings = phase_encodings - -# if isinstance(self.subjects, str): -# self.subjects = [self.subjects] -# if isinstance(self.tasks, str): -# self.tasks = [self.tasks] -# if isinstance(self.phase_encodings, str): -# self.phase_encodings = [self.phase_encodings] - -# if self.tasks is None: -# self.tasks = [ -# 'REST1', -# 'REST2', -# 'SOCIAL', -# 'WM', -# 'RELATIONAL', -# 'EMOTION', -# 'LANGUAGE', -# 'GAMBLING', -# 'MOTOR', -# ] - -# if self.phase_encodings is None: -# self.phase_encodings = ["LR", "RL"] - -# def get_elements(self): -# """Get the list of subjects in the dataset. - -# Returns -# ------- -# elements : list[str] -# The list of subjects in the dataset. -# """ -# elems = [] - -# if self.subjects is None: -# self.subjects = os.listdir(self.datadir) - -# for subject, task, phase_encoding in product( -# self.subjects, self.tasks, self.phase_encodings -# ): -# elems.append((subject, task, phase_encoding)) - -# return elems - -# def __getitem__(self, element): -# """Index one element in the dataset. - -# Parameters -# ---------- -# element : tuple[str, str] -# The element to be indexed. First element in the tuple is the -# subject, second element is the task, third element is the -# phase encoding direction. - -# Returns -# ------- -# out : dict[str -> Path] -# Dictionary of paths for each type of data required for the -# specified element. -# """ -# sub, task, phase_encoding = element -# out = {} - -# if 'REST' in task: -# task_name = f'rfMRI_{task}' -# else: -# task_name = f'tfMRI_{task}' - -# out['BOLD'] = dict( -# path=self.datadir / sub / 'MNINonLinear' / 'Results' / -# f'{task_name}_{phase_encoding}' / -# f'{task_name}_{phase_encoding}_hp2000_clean.nii.gz' -# ) - -# conf_dir = ( -# Path('/data') / 'group' / 'appliedml' / -# 'data' / 'HCP1200_Confounds_tsv' -# ) -# out['BOLD']['confounds'] = ( -# conf_dir / sub / 'MNINonLinear' / 'Results' / -# f'{task_name}_{phase_encoding}' / f'Confounds_{sub}.tsv' -# ) - -# out['meta'] = dict(datagrabber=self.get_meta()) -# self._dataset_get(out) - -# out['meta']['element'] = dict( -# subject=sub, task=task, phase_encoding=phase_encoding -# ) - -# return out diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 894eaf793..87e1e863f 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -14,7 +14,7 @@ configure_logging(level='DEBUG') def test_juselessdataladukbvbm_datagrabber(): with JuselessDataladUKBVBM() as dg: - out = dg[('sub-2670511', 'ses-2')] + out = dg[('2670511', '2')] assert 'VBM_GM' in out assert out['VBM_GM'].name == 'm0wp1sub-2670511_ses-2_T1w.nii.gz' assert out['VBM_GM'].exists() diff --git a/junifer/datagrabber/base.py b/junifer/datagrabber/base.py index 1e8ac9c8c..431f7ca4d 100644 --- a/junifer/datagrabber/base.py +++ b/junifer/datagrabber/base.py @@ -2,6 +2,7 @@ # Leonard Sasse # License: AGPL from pathlib import Path +import re import tempfile import datalad.api as dl @@ -200,8 +201,73 @@ class PatternDataGrabber(BaseDataGrabber): self.patterns = patterns self.replacements = replacements - def _replace_patterns(self, element, pattern): - """Replace the patterns in the pattern with the element. + def _replace_patterns_regex(self, pattern): + """Replace the patterns in the pattern with the named groups so the + elements can be obtained from the filesystem. + + Parameters + ---------- + pattern : str + The pattern to be replaced. + + Returns + ------- + re_pattern : str + The regular expression with the named groups. + glob_pattern : str + The search pattern to be used with glob + + """ + re_pattern = pattern + glob_pattern = pattern + for t_r in self.replacements: + # Replace the first of each with a named group definition + re_pattern = re_pattern.replace( + f'{{{t_r}}}', f'(?P<{t_r}>.*)', 1) + + for t_r in self.replacements: + # Replace the second appearance of each with the named group + # back reference + re_pattern = re_pattern.replace(f'{{{t_r}}}', f'(?P={t_r})') + + for t_r in self.replacements: + glob_pattern = glob_pattern.replace(f'{{{t_r}}}', f'*') + return re_pattern, glob_pattern + + def get_elements(self): + """Get the list of elements in the dataset. It will use regex + to search for `replacements` in the `patterns` and return the + intersection of the results for each type. That is, build a list + of elements that have all the required types. + + Returns + ------- + elements : list + The list of elements in the dataset. + """ + elements = None + for t_type in self.types: + types_element = set() + t_pattern = self.patterns[t_type] # get the pattern + re_pattern, glob_pattern = self._replace_patterns_regex(t_pattern) + for fname in self.datadir.glob(glob_pattern): + suffix = fname.relative_to(self.datadir).as_posix() + m = re.match(re_pattern, suffix) + if m is not None: + t_element = tuple(m.group(k) for k in self.replacements) + if len(self.replacements) == 1: + t_element = t_element[0] + types_element.add(t_element) + if elements is None: + elements = types_element + else: + elements = elements.intersection(types_element) + + return list(elements) + + def _replace_patterns_glob(self, element, pattern): + """Replace the patterns in the pattern with the element so it can + be globbed. Parameters ---------- @@ -247,7 +313,7 @@ class PatternDataGrabber(BaseDataGrabber): element = (element,) for t_type in self.types: t_pattern = self.patterns[t_type] # type: ignore - t_replace = self._replace_patterns(element, t_pattern) + t_replace = self._replace_patterns_glob(element, t_pattern) if '*' in t_replace: t_matches = list(self.datadir.glob(t_replace)) if len(t_matches) > 1: diff --git a/junifer/datagrabber/tests/test_base_datagrabber.py b/junifer/datagrabber/tests/test_base_datagrabber.py index 2a4df25d7..689dd24a6 100644 --- a/junifer/datagrabber/tests/test_base_datagrabber.py +++ b/junifer/datagrabber/tests/test_base_datagrabber.py @@ -12,6 +12,10 @@ _testing_dataset = { 'example_bids': { 'uri': 'https://gin.g-node.org/juaml/datalad-example-bids', 'id': 'e2ce149bd723088769a86c72e57eded009258c6b' + }, + 'example_bids_ses': { + 'uri': 'https://gin.g-node.org/juaml/datalad-example-bids-ses', + 'id': '3d08d55d1faad4f12ab64ac9497544a0d924d47a' } } @@ -119,22 +123,17 @@ def test_bids_datalad_PatternDataGrabber(): } replacements = ['subject'] - class MyDataGrabber(PatternDataladDataGrabber): - def get_elements(self): - elems = [x.name for x in self.datadir.iterdir() if x.is_dir()] - return elems - with pytest.raises(ValueError, match=r"uri must be provided"): - MyDataGrabber(datadir=None, types=types, patterns=patterns, - replacements=replacements) + PatternDataladDataGrabber(datadir=None, types=types, patterns=patterns, + replacements=replacements) repo_uri = _testing_dataset['example_bids']['uri'] rootdir = 'example_bids' repo_commit = _testing_dataset['example_bids']['id'] - with MyDataGrabber(rootdir=rootdir, uri=repo_uri, - types=types, patterns=patterns, - replacements=replacements) as dg: + with PatternDataladDataGrabber( + rootdir=rootdir, uri=repo_uri, types=types, patterns=patterns, + replacements=replacements) as dg: subs = [x for x in dg] expected_subs = [f'sub-{i:02d}' for i in range(1, 10)] assert set(subs) == set(expected_subs) @@ -152,7 +151,7 @@ def test_bids_datalad_PatternDataGrabber(): assert 'datagrabber' in t_sub['meta'] dg_meta = t_sub['meta']['datagrabber'] assert 'class' in dg_meta - assert dg_meta['class'] == 'MyDataGrabber' + assert dg_meta['class'] == 'PatternDataladDataGrabber' assert 'uri' in dg_meta assert dg_meta['uri'] == repo_uri assert 'dataset_commit_id' in dg_meta @@ -167,9 +166,9 @@ def test_bids_datalad_PatternDataGrabber(): 'T1w': '{subject}/anat/{subject}_T*w.nii.gz', 'bold': '{subject}/func/{subject}_task-rest_*.nii.gz' } - with MyDataGrabber(rootdir=rootdir, uri=repo_uri, - types=types, patterns=patterns, - datadir=datadir, replacements=replacements) as dg: + with PatternDataladDataGrabber( + rootdir=rootdir, uri=repo_uri, types=types, patterns=patterns, + datadir=datadir, replacements=replacements) as dg: assert dg.datadir == datadir / rootdir for elem in dg: t_sub = dg[elem] @@ -179,3 +178,44 @@ def test_bids_datalad_PatternDataGrabber(): assert 'path' in t_sub['bold'] assert t_sub['bold']['path'] == \ (dg.datadir / f'{elem}/func/{elem}_task-rest_bold.nii.gz') + + +def test_bids_datalad_PatternDataGrabber_session(): + """Test a subject and session-based BIDS datalad datagrabber""" + types = ['T1w', 'bold'] + patterns = { + 'T1w': '{subject}/{session}/anat/{subject}_{session}_T1w.nii.gz', + 'bold': '{subject}/{session}/func/' + '{subject}_{session}_task-rest_bold.nii.gz' + } + replacements = ['subject', 'session'] + + with pytest.raises(ValueError, match=r"uri must be provided"): + PatternDataladDataGrabber(datadir=None, types=types, patterns=patterns, + replacements=replacements) + + repo_uri = _testing_dataset['example_bids_ses']['uri'] + rootdir = 'example_bids_ses' + # repo_commit = _testing_dataset['example_bids_ses']['id'] + + # With T1W and bold, only 2 sessions are available + with PatternDataladDataGrabber( + rootdir=rootdir, uri=repo_uri, types=types, patterns=patterns, + replacements=replacements) as dg: + subs = [x for x in dg] + expected_subs = [(f'sub-{i:02d}', f'ses-{j:02d}') for j in range(1, 3) + for i in range(1, 10)] + assert set(subs) == set(expected_subs) + + # Test with a different T1w only, it should have 3 sessions + types = ['T1w'] + patterns = { + 'T1w': '{subject}/{session}/anat/{subject}_{session}_T1w.nii.gz', + } + with PatternDataladDataGrabber( + rootdir=rootdir, uri=repo_uri, types=types, patterns=patterns, + replacements=replacements) as dg: + subs = [x for x in dg] + expected_subs = [(f'sub-{i:02d}', f'ses-{j:02d}') for j in range(1, 4) + for i in range(1, 10)] + assert set(subs) == set(expected_subs) diff --git a/tools/create_bids_example_dataset_sessions.py b/tools/create_bids_example_dataset_sessions.py new file mode 100644 index 000000000..3301c6f8a --- /dev/null +++ b/tools/create_bids_example_dataset_sessions.py @@ -0,0 +1,41 @@ +# Authors: Federico Raimondo +# License: AGPL +from tempfile import TemporaryDirectory +from pathlib import Path + +import datalad.api as dl + +dst = 'git@gin.g-node.org:/juaml/datalad-example-bids-ses.git' + +with TemporaryDirectory() as tmpdir_name: + tmpdir = Path(tmpdir_name) + ds = dl.create(tmpdir) # type: ignore + + base_dir = tmpdir / 'example_bids_ses' + base_dir.mkdir() + + for i_sub in range(1, 10): + t_sub = f'sub-{i_sub:02d}' + sub_dir = base_dir / t_sub + sub_dir.mkdir() + + for i_ses in range(1, 4): + t_ses = f'ses-{i_ses:02d}' + ses_dir = sub_dir / t_ses + ses_dir.mkdir() + + for dname in ['anat', 'func']: + (ses_dir / dname).mkdir() + + fnames = [f'anat/{t_sub}_{t_ses}_T1w.nii.gz'] + if i_ses != 3: # Session 3 does not have functional data + fnames.extend([ + f'func/{t_sub}_{t_ses}_task-rest_bold.nii.gz', + f'func/{t_sub}_{t_ses}_task-rest_bold.json']) + for fname in fnames: + with open(ses_dir / fname, 'w') as f: + f.write('placeholder') + + ds.save(recursive=True) + ds.siblings('add', name='gin', url=dst) + ds.push(to='gin', force='all') -- 2.52.0 From 9bc310af4ca380f0605991129ff6913c631dee2b Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 13 Apr 2022 13:06:25 +0300 Subject: [PATCH 35/40] Better juseless test --- junifer/configs/tests/test_juseless.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 87e1e863f..48cdcb275 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -14,9 +14,12 @@ configure_logging(level='DEBUG') def test_juselessdataladukbvbm_datagrabber(): with JuselessDataladUKBVBM() as dg: - out = dg[('2670511', '2')] + all_elements = dg.get_elements() + test_element = all_elements[0] + out = dg[test_element] assert 'VBM_GM' in out - assert out['VBM_GM'].name == 'm0wp1sub-2670511_ses-2_T1w.nii.gz' + assert out['VBM_GM'].name == \ + f'm0wp1sub-{test_element[0]}_ses-{test_element[1]}_T1w.nii.gz' assert out['VBM_GM'].exists() -- 2.52.0 From fc7cab2a306e5dd4680813176f91f4e6888f3c13 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 13 Apr 2022 12:17:56 +0200 Subject: [PATCH 36/40] JuselessUKBVBM data grabber tested --- junifer/configs/juseless.py | 2 +- junifer/configs/tests/test_juseless.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/junifer/configs/juseless.py b/junifer/configs/juseless.py index be3b427f5..db88315b7 100644 --- a/junifer/configs/juseless.py +++ b/junifer/configs/juseless.py @@ -25,7 +25,7 @@ class JuselessDataladUKBVBM(PatternDataladDataGrabber): types = ['VBM_GM'] replacements = ['subject', 'session'] patterns = { - 'VBM_GM': 'm0wp1_sub-{subject}_ses-{session}_T1w.nii.gz' + 'VBM_GM': 'm0wp1sub-{subject}_ses-{session}_T1w.nii.gz' } super().__init__( types=types, datadir=datadir, uri=uri, rootdir=rootdir, diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 48cdcb275..005173806 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -18,9 +18,9 @@ def test_juselessdataladukbvbm_datagrabber(): test_element = all_elements[0] out = dg[test_element] assert 'VBM_GM' in out - assert out['VBM_GM'].name == \ + assert out['VBM_GM']['path'].name == \ f'm0wp1sub-{test_element[0]}_ses-{test_element[1]}_T1w.nii.gz' - assert out['VBM_GM'].exists() + assert out['VBM_GM']['path'].exists() def test_juselessdataladhcp_datagrabber(): @@ -30,5 +30,5 @@ def test_juselessdataladhcp_datagrabber(): out = dg[test_element] - assert out['BOLD'].exists() - assert os.path.isfile(out['BOLD']['path']) + assert out['BOLD']['path'].exists() + assert out['BOLD']['path'].isfile() -- 2.52.0 From dad7ce69fd31d528fcf24deb9589d0bacee2a841 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 13 Apr 2022 12:19:19 +0200 Subject: [PATCH 37/40] Flake --- junifer/configs/tests/test_juseless.py | 1 + 1 file changed, 1 insertion(+) diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 005173806..4fda2f849 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -23,6 +23,7 @@ def test_juselessdataladukbvbm_datagrabber(): assert out['VBM_GM']['path'].exists() + def test_juselessdataladhcp_datagrabber(): with DataladHCP1200() as dg: all_elements = dg.get_elements() -- 2.52.0 From dd0f8192d98cdc94767a7f3e156dddb6bbd2f30f Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Wed, 13 Apr 2022 12:19:50 +0200 Subject: [PATCH 38/40] flake again --- junifer/configs/tests/test_juseless.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/junifer/configs/tests/test_juseless.py b/junifer/configs/tests/test_juseless.py index 4fda2f849..9a266c19c 100644 --- a/junifer/configs/tests/test_juseless.py +++ b/junifer/configs/tests/test_juseless.py @@ -1,6 +1,5 @@ import socket import pytest -import os from junifer.configs.juseless import JuselessDataladUKBVBM from junifer.utils.logging import configure_logging @@ -23,7 +22,6 @@ def test_juselessdataladukbvbm_datagrabber(): assert out['VBM_GM']['path'].exists() - def test_juselessdataladhcp_datagrabber(): with DataladHCP1200() as dg: all_elements = dg.get_elements() -- 2.52.0 From cfd8715bab199cadbb4fb047268d7c0d399cb1fc Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 14 Apr 2022 09:38:35 +0300 Subject: [PATCH 39/40] Use kind in name for storage --- junifer/markers/base.py | 6 ++++-- junifer/markers/tests/test_base_marker.py | 8 ++++---- junifer/markers/tests/test_parcel.py | 8 ++++---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/junifer/markers/base.py b/junifer/markers/base.py index 0471744d0..0cfec4fc6 100644 --- a/junifer/markers/base.py +++ b/junifer/markers/base.py @@ -82,8 +82,10 @@ class BaseMarker(PipelineStepMixin): def get_meta(self, kind): s_meta = super().get_meta() - s_meta['name'] = self.name - s_meta['kind'] = kind # same marker can be fit into different kinds + # same marker can be fit into different kinds, so the name + # is created from the kind and the name of the marker + s_meta['name'] = f'{kind}_{self.name}' + s_meta['kind'] = kind return dict(marker=s_meta) def validate_input(self, input): diff --git a/junifer/markers/tests/test_base_marker.py b/junifer/markers/tests/test_base_marker.py index f661af6df..a0d43c81c 100644 --- a/junifer/markers/tests/test_base_marker.py +++ b/junifer/markers/tests/test_base_marker.py @@ -22,12 +22,12 @@ def test_meta(): t_meta = base.get_meta('bold') assert t_meta['marker']['class'] == 'BaseMarker' - assert t_meta['marker']['name'] == 'BaseMarker' + assert t_meta['marker']['name'] == 'bold_BaseMarker' base = BaseMarker(on=['bold', 'dwi'], name='mymarker') t_meta = base.get_meta('dwi') - assert t_meta['marker']['name'] == 'mymarker' + assert t_meta['marker']['name'] == 'dwi_mymarker' def test_BaseMarker(): @@ -50,12 +50,12 @@ def test_BaseMarker(): out = base.fit_transform(input) assert out['bold']['data'] == 1 - assert out['bold']['meta']['marker']['name'] == 'mymarker' + assert out['bold']['meta']['marker']['name'] == 'bold_mymarker' assert out['bold']['meta']['marker']['class'] == 'BaseMarker' base2 = BaseMarker(on='bold', name='mymarker') base2.compute = lambda x: dict(data=1) # type: ignore out2 = base2.fit_transform(input) assert out2['bold']['data'] == 1 - assert out2['bold']['meta']['marker']['name'] == 'mymarker' + assert out2['bold']['meta']['marker']['name'] == 'bold_mymarker' assert out2['bold']['meta']['marker']['class'] == 'BaseMarker' diff --git a/junifer/markers/tests/test_parcel.py b/junifer/markers/tests/test_parcel.py index fae7313ab..5098f9b01 100644 --- a/junifer/markers/tests/test_parcel.py +++ b/junifer/markers/tests/test_parcel.py @@ -64,7 +64,7 @@ def test_ParcelAggregation_3D(): meta = marker.get_meta('VBM_GM')['marker'] assert meta['method'] == 'mean' assert meta['atlas'] == 'Schaefer100x7' - assert meta['name'] == 'gmd_schaefer100x7_mean' + assert meta['name'] == 'VBM_GM_gmd_schaefer100x7_mean' assert meta['class'] == 'ParcelAggregation' assert meta['kind'] == 'VBM_GM' assert meta['method_params'] == {} @@ -88,7 +88,7 @@ def test_ParcelAggregation_3D(): meta = marker.get_meta('VBM_GM')['marker'] assert meta['method'] == 'std' assert meta['atlas'] == 'Schaefer100x7' - assert meta['name'] == 'ParcelAggregation' + assert meta['name'] == 'VBM_GM_ParcelAggregation' assert meta['class'] == 'ParcelAggregation' assert meta['kind'] == 'VBM_GM' assert meta['method_params'] == {} @@ -116,7 +116,7 @@ def test_ParcelAggregation_3D(): meta = marker.get_meta('VBM_GM')['marker'] assert meta['method'] == 'trim_mean' assert meta['atlas'] == 'Schaefer100x7' - assert meta['name'] == 'ParcelAggregation' + assert meta['name'] == 'VBM_GM_ParcelAggregation' assert meta['class'] == 'ParcelAggregation' assert meta['kind'] == 'VBM_GM' assert meta['method_params'] == {'proportiontocut': 0.1} @@ -147,7 +147,7 @@ def test_ParcelAggregation_4D(): meta = marker.get_meta('BOLD')['marker'] assert meta['method'] == 'mean' assert meta['atlas'] == 'Schaefer100x7' - assert meta['name'] == 'ParcelAggregation' + assert meta['name'] == 'BOLD_ParcelAggregation' assert meta['class'] == 'ParcelAggregation' assert meta['kind'] == 'BOLD' assert meta['method_params'] == {} -- 2.52.0 From 3cec900f80a147baf56f7738947303bdccb18dca Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 14 Apr 2022 11:28:43 +0300 Subject: [PATCH 40/40] Add tests + simplify logic in retrieve_tian --- junifer/data/atlases.py | 113 ++++++++++++++----------------- junifer/data/tests/test_atlas.py | 68 ++++++++++++++++++- 2 files changed, 118 insertions(+), 63 deletions(-) diff --git a/junifer/data/atlases.py b/junifer/data/atlases.py index a45fa9943..5d92f2463 100644 --- a/junifer/data/atlases.py +++ b/junifer/data/atlases.py @@ -3,9 +3,8 @@ # License: AGPL from pathlib import Path import io -import os +import tempfile import requests -import wget import shutil import zipfile import numpy as np @@ -49,34 +48,27 @@ for n_rois in range(100, 1001, 100): } for scale in range(1, 5): - for field in ['3T', '7T']: - if field == '7T': - space = 'MNI6thgeneration' - t_name = f'Tian{scale}x{field}x{space}' - _available_atlases[t_name] = { - 'family': 'Tian', - 'scale': scale, - 'magneticfield': field, - 'valid_resolutions': [1.6] - } - else: - for space in ['MNI6thgeneration', 'MNInonlinear2009cAsym']: - if space == 'MNI6thgeneration': - t_name = f'Tian{scale}x{field}x{space}' - _available_atlases[t_name] = { - 'family': 'Tian', - 'scale': scale, - 'magneticfield': field, - 'valid_resolutions': [1, 2] - } - elif space == 'MNInonlinear2009cAsym': - t_name = f'Tian{scale}x{field}x{space}' - _available_atlases[t_name] = { - 'family': 'Tian', - 'scale': scale, - 'magneticfield': field, - 'valid_resolutions': [2] - } + t_name = f'TianxS{scale}x7TxMNI6thgeneration' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': '7T', + 'space': 'MNI6thgeneration' + } + t_name = f'TianxS{scale}x3TxMNI6thgeneration' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': '3T', + 'space': 'MNI6thgeneration' + } + t_name = f'TianxS{scale}x3TxMNInonlinear2009cAsym' + _available_atlases[t_name] = { + 'family': 'Tian', + 'scale': scale, + 'magneticfield': '3T', + 'space': 'MNInonlinear2009cAsym' + } def register_atlas(name, atlas_path, atl_labels, overwrite=False): @@ -255,7 +247,7 @@ def _retrieve_atlas(family, atlas_dir=None, resolution=None, **kwargs): space (optional) : str Space of atlas can be either 'MNI6thgeneration' or 'MNInonlinear2009cAsym' (for some cases). - Defaults to 'MNI6thgeneration'. (For more information see + Defaults to 'MNI6thgeneration'. (For more information see https://github.com/yetianmed/subcortex) magneticfield (optional) : str Options are 3T and 7T, defaults to 3T. @@ -370,7 +362,12 @@ def _retrieve_schaefer(atlas_dir, resolution, n_rois=None, yeo_networks=7): def _retrieve_tian( atlas_dir, resolution, scale=None, space='MNI6thgeneration', magneticfield='3T'): - + # show atlas parameters to user + logger.info('Atlas parameters:') + logger.info(f'\tscale: {scale}') + logger.info(f'\tspace: {space}') + logger.info(f'\tmagneticfield: {magneticfield}') + logger.info(f'\tresolution: {resolution}') # check validity of atlas parameters _valid_scales = [1, 2, 3, 4] _valid_fields = ['3T', '7T'] @@ -378,18 +375,18 @@ def _retrieve_tian( raise_error( f'The parameter `scale` ({scale}) needs to be one of the ' f'following: {_valid_scales}') - if field not in _valid_fields: + if magneticfield not in _valid_fields: raise_error( - f'The parameter `magneticfield` ({field}) needs to be one of ' - f'the following: {_valid_fields}') + f'The parameter `magneticfield` ({magneticfield}) needs to be ' + f'one of the following: {_valid_fields}') if magneticfield == '3T': _valid_spaces = ['MNI6thgeneration', 'MNInonlinear2009cAsym'] if space == 'MNI6thgeneration': _valid_resolutions = [1, 2] - elif space == 'MNInonlinear2009cAsym': + else: # space == 'MNInonlinear2009cAsym': _valid_resolutions = [2] - if magneticfield == '7T': + else: # magneticfield == '7T': _valid_spaces = ['MNI6thgeneration'] _valid_resolutions = [1.6] if space not in _valid_spaces: @@ -399,13 +396,6 @@ def _retrieve_tian( resolution = _closest_resolution(resolution, _valid_resolutions) - # show atlas parameters to user - logger.info('Atlas parameters:') - logger.info(f'\tscale: {scale}') - logger.info(f'\tspace: {space}') - logger.info(f'\tmagneticfield: {magneticfield}') - logger.info(f'\tresolution: {resolution}') - # define file names if magneticfield == '3T': atlas_fname_base_3T = ( @@ -416,14 +406,13 @@ def _retrieve_tian( atlas_fname = atlas_fname_base_3T / ( f'Tian_Subcortex_S{scale}_{magneticfield}.nii.gz') if resolution == 1: - atlas_fname = atlas_fname_base_3T / ( - f'Tian_Subcortex_S{scale}_{magneticfield}_{resolution}' - 'mm.nii.gz') - elif space == 'MNInonlinear2009cAsym': + atlas_fname = atlas_fname_base_3T / \ + f'Tian_Subcortex_S{scale}_{magneticfield}_1mm.nii.gz' + else: # space == 'MNInonlinear2009cAsym': space = '2009cAsym' atlas_fname = atlas_fname_base_3T / ( f'Tian_Subcortex_S{scale}_{magneticfield}_{space}.nii.gz') - elif magneticfield == '7T': + else: # magneticfield == '7T': atlas_fname_base_7T = ( atlas_dir / 'Tian2020MSA_v1.1' / '7T') atlas_fname = atlas_dir / 'Tian2020MSA_v1.1' / f'{magneticfield}' / ( @@ -431,7 +420,7 @@ def _retrieve_tian( # define 7T labels (b/c currently no labels file available for 7T) scale7Trois = {1: 16, 2: 34, 3: 54, 4: 62} labels = [ - ('parcel_' + str(x)) for x in np.arange(1, scale7Trois[scale]+1)] + ('parcel_' + str(x)) for x in np.arange(1, scale7Trois[scale] + 1)] atlas_lname = atlas_fname_base_7T / ( f'Tian_Subcortex_S{scale}_7T_labelnumbering.txt') with open(atlas_lname, 'w') as filehandle: @@ -447,18 +436,20 @@ def _retrieve_tian( 'At least one of the atlas files is missing. ' 'Fetching.') - url_basis = ( - 'https://www.nitrc.org/frs/download.php/12012/Tian2020MSA_v1.1.zip') + url_basis = \ + 'https://www.nitrc.org/frs/download.php/12012/Tian2020MSA_v1.1.zip' - logger.info(f'Downloading {url_basis}') - atlas_download_dir = wget.download(url_basis, atlas_dir.as_posix()) - with zipfile.ZipFile(atlas_download_dir, 'r') as zip_ref: - zip_ref.extractall(atlas_dir.as_posix()) - # clean after unzipping - if os.path.exists(atlas_download_dir): - os.remove(atlas_download_dir) - if os.path.exists((atlas_dir / '__MACOSX').as_posix()): - shutil.rmtree((atlas_dir / '__MACOSX').as_posix()) + logger.info(f'Downloading TIAN from {url_basis}') + with tempfile.TemporaryDirectory() as tmpdir: + atlas_download = requests.get(url_basis) + atlas_zip_fname = Path(tmpdir) / 'Tian2020MSA_v1.1.zip' + with open(atlas_zip_fname, 'wb') as f: + f.write(atlas_download.content) + with zipfile.ZipFile(atlas_zip_fname, 'r') as zip_ref: + zip_ref.extractall(atlas_dir.as_posix()) + # clean after unzipping + if (atlas_dir / '__MACOSX').exists(): + shutil.rmtree((atlas_dir / '__MACOSX').as_posix()) labels = pd.read_csv(atlas_lname, sep=" ", header=None)[0].to_list() diff --git a/junifer/data/tests/test_atlas.py b/junifer/data/tests/test_atlas.py index c02b1b42b..e42c02228 100644 --- a/junifer/data/tests/test_atlas.py +++ b/junifer/data/tests/test_atlas.py @@ -1,11 +1,11 @@ import tempfile import pytest from pathlib import Path -from numpy.testing import assert_array_equal +from numpy.testing import assert_array_equal, assert_array_almost_equal from junifer.data.atlases import (register_atlas, list_atlases, load_atlas, _retrieve_schaefer, _retrieve_suit, - _retrieve_atlas) + _retrieve_atlas, _retrieve_tian) def test_register_atlas(): @@ -150,3 +150,67 @@ def test_suit(): with pytest.raises(ValueError, match=r"The parameter `space`"): _retrieve_suit(tmpdir, 1, space='wrong') + + +def test_tian(): + """Test TIAN atlas""" + + atlases = list_atlases() + assert 'TianxS1x3TxMNI6thgeneration' in atlases + assert 'TianxS2x3TxMNI6thgeneration' in atlases + assert 'TianxS3x3TxMNI6thgeneration' in atlases + assert 'TianxS4x3TxMNI6thgeneration' in atlases + + assert 'TianxS1x3TxMNInonlinear2009cAsym' in atlases + assert 'TianxS2x3TxMNInonlinear2009cAsym' in atlases + assert 'TianxS3x3TxMNInonlinear2009cAsym' in atlases + assert 'TianxS4x3TxMNInonlinear2009cAsym' in atlases + + assert 'TianxS1x7TxMNI6thgeneration' in atlases + assert 'TianxS2x7TxMNI6thgeneration' in atlases + assert 'TianxS3x7TxMNI6thgeneration' in atlases + assert 'TianxS4x7TxMNI6thgeneration' in atlases + + with tempfile.TemporaryDirectory() as tmpdir: + for scale, n_lbl in zip([1, 2, 3, 4], [16, 32, 50, 54]): + img, lbl, fname = load_atlas( + f'TianxS{scale}x3TxMNI6thgeneration', atlas_dir=tmpdir) + fname1 = f'Tian_Subcortex_S{scale}_3T_1mm.nii.gz' + assert img is not None + assert fname.name == fname1 # type: ignore + assert len(lbl) == n_lbl + assert_array_equal(img.header['pixdim'][1:4], [1, 1, 1]) + + img, lbl, fname = load_atlas( + f'TianxS{scale}x3TxMNI6thgeneration', atlas_dir=tmpdir, + resolution=2) + fname1 = f'Tian_Subcortex_S{scale}_3T.nii.gz' + assert img is not None + assert fname.name == fname1 # type: ignore + assert len(lbl) == n_lbl + assert_array_equal(img.header['pixdim'][1:4], [2, 2, 2]) + + img, lbl, fname = load_atlas( + f'TianxS{scale}x3TxMNInonlinear2009cAsym', atlas_dir=tmpdir) + fname1 = f'Tian_Subcortex_S{scale}_3T_2009cAsym.nii.gz' + assert img is not None + assert fname.name == fname1 # type: ignore + assert len(lbl) == n_lbl + assert_array_equal(img.header['pixdim'][1:4], [2, 2, 2]) + + for scale, n_lbl in zip([1, 2, 3, 4], [16, 34, 54, 62]): + img, lbl, fname = load_atlas( + f'TianxS{scale}x7TxMNI6thgeneration', atlas_dir=tmpdir) + fname1 = f'Tian_Subcortex_S{scale}_7T.nii.gz' + assert img is not None + assert fname.name == fname1 # type: ignore + assert len(lbl) == n_lbl + assert_array_almost_equal( + img.header['pixdim'][1:4], [1.6, 1.6, 1.6]) + + with pytest.raises(ValueError, match=r"The parameter `space`"): + _retrieve_tian(tmpdir, resolution=1, scale=1, space='wrong') + + with pytest.raises(ValueError, match=r"The parameter `magneticfield`"): + _retrieve_tian( + tmpdir, resolution=1, scale=1, magneticfield='wrong') -- 2.52.0