diff --git a/docs/changes/newsfragments/341.enh b/docs/changes/newsfragments/341.enh new file mode 100644 index 000000000..ce0c29356 --- /dev/null +++ b/docs/changes/newsfragments/341.enh @@ -0,0 +1 @@ +Refactor DataGrabber ``patterns`` to make helper types like ``*_mask`` as "nested types" of the actual data type by `Synchon Mandal`_ diff --git a/docs/extending/datagrabber.rst b/docs/extending/datagrabber.rst index 5db379caa..94e442c19 100644 --- a/docs/extending/datagrabber.rst +++ b/docs/extending/datagrabber.rst @@ -297,22 +297,22 @@ This approach can be used directly from the YAML, like so: .. code-block:: yaml datagrabber: - - kind: PatternDataladDataGrabber - types: - - BOLD - - T1w - patterns: - BOLD: - pattern: "{subject}/{session}/func/{subject}_{session}_task-rest_bold.nii.gz" - space: MNI152NLin6Asym - T1w: - pattern: "{subject}/{session}/anat/{subject}_{session}_T1w.nii.gz" - space: native - replacements: - - subject - - session - uri: "https://gin.g-node.org/juaml/datalad-example-bids" - rootdir: example_bids_ses + kind: PatternDataladDataGrabber + types: + - BOLD + - T1w + patterns: + BOLD: + pattern: "{subject}/{session}/func/{subject}_{session}_task-rest_bold.nii.gz" + space: MNI152NLin6Asym + T1w: + pattern: "{subject}/{session}/anat/{subject}_{session}_T1w.nii.gz" + space: native + replacements: + - subject + - session + uri: "https://gin.g-node.org/juaml/datalad-example-bids" + rootdir: example_bids_ses .. _extending_datagrabbers_base: @@ -442,11 +442,11 @@ Step 4: Optional: Adding *BOLD confounds* ----------------------------------------- For some analyses, it is useful to have the confounds associated with the BOLD -data. This corresponds to the ``BOLD_confounds`` item in the +data. This corresponds to the ``BOLD.confounds`` item in the :ref:`Data Object ` (see :ref:`data_types`). However, the -``BOLD_confounds`` element does not only consists of a ``path``, but it requires +``BOLD.confounds`` element does not only consists of a ``path``, but it requires more information about the format of the confounds file. Thus, the -``BOLD_confounds`` element is a dictionary with the following keys: +``BOLD.confounds`` element is a dictionary with the following keys: - ``path``: the path to the confounds file. - ``format``: the format of the confounds file. Currently, this can be either @@ -482,15 +482,15 @@ this: "BOLD": { "path": f"{subject}/{session}/func/{subject}_{session}_task-rest_bold.nii.gz", "space": "MNI152NLin6Asym", - }, - "BOLD_confounds": { - "path": f"{subject}/{session}/func/{subject}_{session}_confounds.tsv", - "format": "adhoc", - "mappings": { - "fmriprep": { - "variable1": "rot_x", - "variable2": "rot_z", - "variable3": "rot_y", + "confounds": { + "path": f"{subject}/{session}/func/{subject}_{session}_confounds.tsv", + "format": "adhoc", + "mappings": { + "fmriprep": { + "variable1": "rot_x", + "variable2": "rot_z", + "variable3": "rot_y", + }, }, }, }, diff --git a/docs/extending/marker.rst b/docs/extending/marker.rst index 90a8d0c73..744a4cc2f 100644 --- a/docs/extending/marker.rst +++ b/docs/extending/marker.rst @@ -114,8 +114,7 @@ arguments: path to the data. The dictionary can also contain other keys, depending on the data type. * ``extra_input``: the rest of the :ref:`Data Object`. This is - useful if you want to use other data to compute the Marker - (e.g.: ``BOLD_confounds`` can be used to de-confound the ``BOLD`` data). + useful if you want to use other data to compute the Marker. Following the example, we will compute the mean of the data in each parcel using :class:`nilearn.maskers.NiftiLabelsMasker`. Importantly, the output of the diff --git a/docs/understanding/data.rst b/docs/understanding/data.rst index f71c8bb0c..1d98f801c 100644 --- a/docs/understanding/data.rst +++ b/docs/understanding/data.rst @@ -109,9 +109,6 @@ Data Types * - ``BOLD`` - BOLD image (4D) - Preprocessed or Denoised BOLD image (fMRIPrep output) - * - ``BOLD_confounds`` - - BOLD image confounds (CSV/TSV file) - - Confounds that can be applied to the BOLD image. * - ``VBM_GM`` - VBM Gray Matter segmentation (3D) - CAT output (`m0wp1` images) diff --git a/docs/understanding/preprocess.rst b/docs/understanding/preprocess.rst index afbf28ce9..37b4c3a8a 100644 --- a/docs/understanding/preprocess.rst +++ b/docs/understanding/preprocess.rst @@ -23,7 +23,7 @@ Confound Removal ---------------- This step is meant to remove *confounds* from the ``BOLD`` data. The confounds -are extracted from the ``BOLD_confounds`` data (must be provided by the +are extracted from the ``BOLD.confounds`` data (must be provided by the :ref:`Data Grabber `). The confounds are then regressed out from the ``BOLD`` data using :func:`nilearn.image.clean_img`. @@ -36,7 +36,7 @@ Strategy This confound remover uses the `nilearn`_ API from :func:`nilearn.interfaces.fmriprep.load_confounds`. That is, define a *strategy* -to extract the confounds from the ``BOLD_confounds`` data. The *strategy* is +to extract the confounds from the ``BOLD.confounds`` data. The *strategy* is defined by choosing the *noise components* to be used and the *confounds* to be extracted from each noise components. The *noise components* currently supported are: diff --git a/junifer/api/functions.py b/junifer/api/functions.py index efb987074..5ae749056 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -362,7 +362,7 @@ def reset(config: Dict) -> None: shutil.rmtree(job_dir) # Remove parent directory (if empty) if not next(os.scandir(job_dir.parent), None): - job_dir.parent.rmdir() + job_dir.parent.rmdir() def list_elements( diff --git a/junifer/configs/juseless/datagrabbers/tests/test_ucla.py b/junifer/configs/juseless/datagrabbers/tests/test_ucla.py index 5a2bd7a9c..f5fff921c 100644 --- a/junifer/configs/juseless/datagrabbers/tests/test_ucla.py +++ b/junifer/configs/juseless/datagrabbers/tests/test_ucla.py @@ -27,7 +27,6 @@ def test_JuselessUCLA() -> None: types = [ "BOLD", - "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", @@ -43,12 +42,11 @@ def test_JuselessUCLA() -> None: "types", [ "BOLD", - "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", - ["BOLD", "BOLD_confounds"], + ["BOLD", "VBM_CSF"], ["T1w", "VBM_CSF"], ["VBM_GM", "VBM_WM"], ["BOLD", "T1w"], diff --git a/junifer/configs/juseless/datagrabbers/ucla.py b/junifer/configs/juseless/datagrabbers/ucla.py index ea6f20fca..c81db95da 100644 --- a/junifer/configs/juseless/datagrabbers/ucla.py +++ b/junifer/configs/juseless/datagrabbers/ucla.py @@ -23,8 +23,8 @@ class JuselessUCLA(PatternDataGrabber): datadir : str or Path, optional The directory where the dataset is stored. (default "/data/project/psychosis_thalamus/data/fmriprep"). - types: {"BOLD", "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", \ - "VBM_WM"} or a list of the options, optional + types: {"BOLD", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM"} or \ + list of the options, optional UCLA data types. If None, all available data types are selected. (default None). tasks : {"rest", "bart", "bht", "pamenc", "pamret", \ @@ -76,13 +76,13 @@ class JuselessUCLA(PatternDataGrabber): "MNI152NLin2009cAsym_preproc.nii.gz" ), "space": "MNI152NLin2009cAsym", - }, - "BOLD_confounds": { - "pattern": ( - "{subject}/func/{subject}_" - "task-{task}_bold_confounds.tsv" - ), - "space": "fmriprep", + "confounds": { + "pattern": ( + "{subject}/func/{subject}_" + "task-{task}_bold_confounds.tsv" + ), + "space": "fmriprep", + }, }, "T1w": { "pattern": ( diff --git a/junifer/data/masks.py b/junifer/data/masks.py index 9e893a45b..eec5dbaeb 100644 --- a/junifer/data/masks.py +++ b/junifer/data/masks.py @@ -250,8 +250,8 @@ def get_mask( # noqa: C901 ValueError If extra key is provided in addition to mask name in ``masks`` or if no mask is provided or - if ``masks = "inherit"`` but ``extra_input`` is None or ``mask_item`` - is None or ``mask_items``'s value is not in ``extra_input`` or + if ``masks = "inherit"`` and ``mask`` key for the ``target_data`` is + not found or if callable parameters are passed to non-callable mask or if parameters are passed to :func:`nilearn.masking.intersect_masks` when there is only one mask or @@ -310,8 +310,8 @@ def get_mask( # noqa: C901 if len(true_masks) == 0: raise_error("No mask was passed. At least one mask is required.") - # Get the data type for the input data type's mask - inherited_mask_item = target_data.get("mask_item", None) + # Get the nested mask data type for the input data type + inherited_mask_item = target_data.get("mask", None) # Create component-scoped tempdir tempdir = WorkDirManager().get_tempdir(prefix="masks") @@ -330,28 +330,16 @@ def get_mask( # noqa: C901 mask_name = t_mask mask_params = None - # If mask is being inherited from previous steps like preprocessing + # If mask is being inherited from the datagrabber or a preprocessor, + # check that it's accessible if mask_name == "inherit": - # Requires extra input to be passed - if extra_input is None: - raise_error( - "Cannot inherit mask from another data item " - "because no extra data was passed." - ) - # Missing inherited mask item if inherited_mask_item is None: raise_error( - "Cannot inherit mask from another data item " - "because no mask item was specified " - "(missing `mask_item` key in the data object)." + "Cannot inherit mask from the target data. Either the " + "DataGrabber or a Preprocessor does not provide `mask` " + "for the target data type." ) - # Missing inherited mask item in extra input - if inherited_mask_item not in extra_input: - raise_error( - "Cannot inherit mask from another data item " - f"because the item ({inherited_mask_item}) does not exist." - ) - mask_img = extra_input[inherited_mask_item]["data"] + mask_img = inherited_mask_item["data"] # Starting with new mask else: # Load mask diff --git a/junifer/data/parcellations.py b/junifer/data/parcellations.py index 947c3acbc..cfd242659 100644 --- a/junifer/data/parcellations.py +++ b/junifer/data/parcellations.py @@ -889,7 +889,7 @@ def _retrieve_tian( ) with open(parcellation_lname, "w") as filehandle: for listitem in labels: - filehandle.write("%s\n" % listitem) + filehandle.write(f"{listitem}\n") logger.info( "Currently there are no labels provided for the 7T Tian " "parcellation. A simple numbering scheme for distinction was " diff --git a/junifer/data/tests/test_masks.py b/junifer/data/tests/test_masks.py index c8da1298e..2b079bdfb 100644 --- a/junifer/data/tests/test_masks.py +++ b/junifer/data/tests/test_masks.py @@ -365,27 +365,10 @@ def test_get_mask_errors() -> None: target_data=vbm_gm, ) - # Test "inherited" masks errors - - # 1) No extra_data parameter - with pytest.raises(ValueError, match=r"no extra data was passed"): + # Test "inherited" masks error + with pytest.raises(ValueError, match=r"provide `mask`"): get_mask(masks="inherit", target_data=vbm_gm) - extra_input = {"VBM_MASK": {}} - - # 2) No mask_item key in target_data - with pytest.raises(ValueError, match=r"no mask item was specified"): - get_mask( - masks="inherit", target_data=vbm_gm, extra_input=extra_input - ) - - # 3) mask_item not in extra data - with pytest.raises(ValueError, match=r"does not exist"): - vbm_gm["mask_item"] = "wrong" - get_mask( - masks="inherit", target_data=vbm_gm, extra_input=extra_input - ) - @pytest.mark.parametrize( "mask_name,function,params,resample", @@ -457,18 +440,15 @@ def test_get_mask_inherit() -> None: ) # Now get the mask using the inherit functionality, passing the - # computed mask as extra data - extra_input = { - "BOLD_MASK": { - "data": gm_mask, - "space": element_data["BOLD"]["space"], - } + # computed mask as the data + bold_dict = element_data["BOLD"] + bold_dict["mask"] = { + "data": gm_mask, + "space": element_data["BOLD"]["space"], } - element_data["BOLD"]["mask_item"] = "BOLD_MASK" mask2 = get_mask( masks="inherit", - target_data=element_data["BOLD"], - extra_input=extra_input, + target_data=bold_dict, ) # Both masks should be equal diff --git a/junifer/datagrabber/aomic/id1000.py b/junifer/datagrabber/aomic/id1000.py index 4af634252..ae16d0ef9 100644 --- a/junifer/datagrabber/aomic/id1000.py +++ b/junifer/datagrabber/aomic/id1000.py @@ -24,9 +24,8 @@ class DataladAOMICID1000(PatternDataladDataGrabber): The directory where the datalad dataset will be cloned. If None, the datalad dataset will be cloned into a temporary directory (default None). - types: {"BOLD", "BOLD_confounds", "BOLD_mask", "T1w", "T1w_mask", \ - "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ - a list of the options, optional + types: {"BOLD", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ + list of the options, optional AOMIC data types. If None, all available data types are selected. (default None). native_t1w : bool, optional @@ -49,24 +48,23 @@ class DataladAOMICID1000(PatternDataladDataGrabber): "space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "BOLD_mask", - }, - "BOLD_confounds": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-moviewatching_" - "desc-confounds_regressors.tsv" - ), - "format": "fmriprep", - }, - "BOLD_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-moviewatching_" - "space-MNI152NLin2009cAsym_" - "desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-moviewatching_" + "space-MNI152NLin2009cAsym_" + "desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, + "confounds": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-moviewatching_" + "desc-confounds_regressors.tsv" + ), + "format": "fmriprep", + }, }, "T1w": { "pattern": ( @@ -75,15 +73,14 @@ class DataladAOMICID1000(PatternDataladDataGrabber): "desc-preproc_T1w.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_space-MNI152NLin2009cAsym_" - "desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_space-MNI152NLin2009cAsym_" + "desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, }, "VBM_CSF": { "pattern": ( @@ -128,14 +125,13 @@ class DataladAOMICID1000(PatternDataladDataGrabber): "{subject}_desc-preproc_T1w.nii.gz" ), "space": "native", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_desc-brain_mask.nii.gz" - ), - "space": "native", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_desc-brain_mask.nii.gz" + ), + "space": "native", + }, }, "Warp": { "pattern": ( diff --git a/junifer/datagrabber/aomic/piop1.py b/junifer/datagrabber/aomic/piop1.py index f6b6afc1a..2ebe94c5a 100644 --- a/junifer/datagrabber/aomic/piop1.py +++ b/junifer/datagrabber/aomic/piop1.py @@ -26,9 +26,8 @@ class DataladAOMICPIOP1(PatternDataladDataGrabber): The directory where the datalad dataset will be cloned. If None, the datalad dataset will be cloned into a temporary directory (default None). - types: {"BOLD", "BOLD_confounds", "BOLD_mask", "T1w", "T1w_mask", \ - "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ - a list of the options, optional + types: {"BOLD", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ + list of the options, optional AOMIC data types. If None, all available data types are selected. (default None). tasks : {"restingstate", "anticipation", "emomatching", "faces", \ @@ -85,23 +84,22 @@ class DataladAOMICPIOP1(PatternDataladDataGrabber): "space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "BOLD_mask", - }, - "BOLD_confounds": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-{task}_" - "desc-confounds_regressors.tsv" - ), - "format": "fmriprep", - }, - "BOLD_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-{task}_" - "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-{task}_" + "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, + "confounds": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-{task}_" + "desc-confounds_regressors.tsv" + ), + "format": "fmriprep", + }, }, "T1w": { "pattern": ( @@ -110,15 +108,14 @@ class DataladAOMICPIOP1(PatternDataladDataGrabber): "desc-preproc_T1w.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_space-MNI152NLin2009cAsym_" - "desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_space-MNI152NLin2009cAsym_" + "desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, }, "VBM_CSF": { "pattern": ( @@ -163,14 +160,13 @@ class DataladAOMICPIOP1(PatternDataladDataGrabber): "{subject}_desc-preproc_T1w.nii.gz" ), "space": "native", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_desc-brain_mask.nii.gz" - ), - "space": "native", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_desc-brain_mask.nii.gz" + ), + "space": "native", + }, }, "Warp": { "pattern": ( diff --git a/junifer/datagrabber/aomic/piop2.py b/junifer/datagrabber/aomic/piop2.py index 6f25444da..9fd6cbc23 100644 --- a/junifer/datagrabber/aomic/piop2.py +++ b/junifer/datagrabber/aomic/piop2.py @@ -26,13 +26,12 @@ class DataladAOMICPIOP2(PatternDataladDataGrabber): The directory where the datalad dataset will be cloned. If None, the datalad dataset will be cloned into a temporary directory (default None). - types: {"BOLD", "BOLD_confounds", "BOLD_mask", "T1w", "T1w_mask", \ - "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ - a list of the options, optional + types: {"BOLD", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI"} or \ + list of the options, optional AOMIC data types. If None, all available data types are selected. (default None). - tasks : {"restingstate", "stopsignal", "workingmemory"} \ - or list of the options, optional + tasks : {"restingstate", "stopsignal", "workingmemory"} or \ + list of the options, optional AOMIC PIOP2 task sessions. If None, all available task sessions are selected (default None). native_t1w : bool, optional @@ -82,23 +81,22 @@ class DataladAOMICPIOP2(PatternDataladDataGrabber): "space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "BOLD_mask", - }, - "BOLD_confounds": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-{task}_" - "desc-confounds_regressors.tsv" - ), - "format": "fmriprep", - }, - "BOLD_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/func/" - "{subject}_task-{task}_" - "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-{task}_" + "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, + "confounds": { + "pattern": ( + "derivatives/fmriprep/{subject}/func/" + "{subject}_task-{task}_" + "desc-confounds_regressors.tsv" + ), + "format": "fmriprep", + }, }, "T1w": { "pattern": ( @@ -107,15 +105,14 @@ class DataladAOMICPIOP2(PatternDataladDataGrabber): "desc-preproc_T1w.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_space-MNI152NLin2009cAsym_" - "desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_space-MNI152NLin2009cAsym_" + "desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, }, "VBM_CSF": { "pattern": ( @@ -160,14 +157,13 @@ class DataladAOMICPIOP2(PatternDataladDataGrabber): "{subject}_desc-preproc_T1w.nii.gz" ), "space": "native", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep/{subject}/anat/" - "{subject}_desc-brain_mask.nii.gz" - ), - "space": "native", + "mask": { + "pattern": ( + "derivatives/fmriprep/{subject}/anat/" + "{subject}_desc-brain_mask.nii.gz" + ), + "space": "native", + }, }, "Warp": { "pattern": ( diff --git a/junifer/datagrabber/aomic/tests/test_id1000.py b/junifer/datagrabber/aomic/tests/test_id1000.py index 6ffd3ce8d..248753d88 100644 --- a/junifer/datagrabber/aomic/tests/test_id1000.py +++ b/junifer/datagrabber/aomic/tests/test_id1000.py @@ -40,20 +40,20 @@ def test_DataladAOMICID1000() -> None: assert out["BOLD"]["path"].exists() assert out["BOLD"]["path"].is_file() - # asserts type "BOLD_confounds" - assert "BOLD_confounds" in out + # asserts type BOLD.confounds + assert "confounds" in out["BOLD"] assert ( - out["BOLD_confounds"]["path"].name + out["BOLD"]["confounds"]["path"].name == f"{test_element}_task-moviewatching_" "desc-confounds_regressors.tsv" ) - assert out["BOLD_confounds"]["path"].exists() - assert out["BOLD_confounds"]["path"].is_file() + assert out["BOLD"]["confounds"]["path"].exists() + assert out["BOLD"]["confounds"]["path"].is_file() - # assert BOLD_mask - assert out["BOLD_mask"]["path"].exists() + # assert BOLD.mask + assert out["BOLD"]["mask"]["path"].exists() # asserts type "T1w" assert "T1w" in out @@ -67,8 +67,8 @@ def test_DataladAOMICID1000() -> None: assert out["T1w"]["path"].exists() assert out["T1w"]["path"].is_file() - # asserts T1w_mask - assert out["T1w_mask"]["path"].exists() + # asserts T1w.mask + assert out["T1w"]["mask"]["path"].exists() # asserts type "VBM_CSF" assert "VBM_CSF" in out @@ -129,13 +129,12 @@ def test_DataladAOMICID1000() -> None: "types", [ "BOLD", - "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI", - ["BOLD", "BOLD_confounds"], + ["BOLD", "VBM_CSF"], ["T1w", "VBM_CSF"], ["VBM_GM", "VBM_WM"], ["DWI", "BOLD"], diff --git a/junifer/datagrabber/aomic/tests/test_piop1.py b/junifer/datagrabber/aomic/tests/test_piop1.py index fc4f864f2..dfb79219f 100644 --- a/junifer/datagrabber/aomic/tests/test_piop1.py +++ b/junifer/datagrabber/aomic/tests/test_piop1.py @@ -63,19 +63,19 @@ def test_DataladAOMICPIOP1(tasks: Optional[str]) -> None: assert out["BOLD"]["path"].exists() assert out["BOLD"]["path"].is_file() - # asserts type "BOLD_confounds" - assert "BOLD_confounds" in out + # asserts type BOLD.confounds + assert "confounds" in out["BOLD"] assert ( - out["BOLD_confounds"]["path"].name == f"{sub}_task-{new_task}_" + out["BOLD"]["confounds"]["path"].name == f"{sub}_task-{new_task}_" "desc-confounds_regressors.tsv" ) - assert out["BOLD_confounds"]["path"].exists() - assert out["BOLD_confounds"]["path"].is_file() + assert out["BOLD"]["confounds"]["path"].exists() + assert out["BOLD"]["confounds"]["path"].is_file() - # assert BOLD_mask - assert out["BOLD_mask"]["path"].exists() + # assert BOLD.mask + assert out["BOLD"]["mask"]["path"].exists() # asserts type "T1w" assert "T1w" in out @@ -88,8 +88,8 @@ def test_DataladAOMICPIOP1(tasks: Optional[str]) -> None: assert out["T1w"]["path"].exists() assert out["T1w"]["path"].is_file() - # asserts T1w_mask - assert out["T1w_mask"]["path"].exists() + # asserts T1w.mask + assert out["T1w"]["mask"]["path"].exists() # asserts type "VBM_CSF" assert "VBM_CSF" in out @@ -147,13 +147,12 @@ def test_DataladAOMICPIOP1(tasks: Optional[str]) -> None: "types", [ "BOLD", - "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI", - ["BOLD", "BOLD_confounds"], + ["BOLD", "VBM_CSF"], ["T1w", "VBM_CSF"], ["VBM_GM", "VBM_WM"], ["DWI", "BOLD"], diff --git a/junifer/datagrabber/aomic/tests/test_piop2.py b/junifer/datagrabber/aomic/tests/test_piop2.py index cd8ffae56..b316fdf3d 100644 --- a/junifer/datagrabber/aomic/tests/test_piop2.py +++ b/junifer/datagrabber/aomic/tests/test_piop2.py @@ -57,19 +57,19 @@ def test_DataladAOMICPIOP2(tasks: Optional[str]) -> None: assert out["BOLD"]["path"].exists() assert out["BOLD"]["path"].is_file() - # asserts type "BOLD_confounds" - assert "BOLD_confounds" in out + # asserts type BOLD.confounds + assert "confounds" in out["BOLD"] assert ( - out["BOLD_confounds"]["path"].name == f"{sub}_task-{new_task}_" + out["BOLD"]["confounds"]["path"].name == f"{sub}_task-{new_task}_" "desc-confounds_regressors.tsv" ) - assert out["BOLD_confounds"]["path"].exists() - assert out["BOLD_confounds"]["path"].is_file() + assert out["BOLD"]["confounds"]["path"].exists() + assert out["BOLD"]["confounds"]["path"].is_file() - # assert BOLD_mask - assert out["BOLD_mask"]["path"].exists() + # assert BOLD.mask + assert out["BOLD"]["mask"]["path"].exists() # asserts type "T1w" assert "T1w" in out @@ -82,8 +82,8 @@ def test_DataladAOMICPIOP2(tasks: Optional[str]) -> None: assert out["T1w"]["path"].exists() assert out["T1w"]["path"].is_file() - # asserts T1w_mask - assert out["T1w_mask"]["path"].exists() + # asserts T1w.mask + assert out["T1w"]["mask"]["path"].exists() # asserts type "VBM_CSF" assert "VBM_CSF" in out @@ -141,13 +141,12 @@ def test_DataladAOMICPIOP2(tasks: Optional[str]) -> None: "types", [ "BOLD", - "BOLD_confounds", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM", "DWI", - ["BOLD", "BOLD_confounds"], + ["BOLD", "VBM_CSF"], ["T1w", "VBM_CSF"], ["VBM_GM", "VBM_WM"], ["DWI", "BOLD"], diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index 390ef90c9..3680ec8ea 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -176,7 +176,16 @@ class DataladDataGrabber(BaseDataGrabber): The unmodified input dictionary. """ - to_get = [v["path"] for v in out.values() if "path" in v] + to_get = [] + for type_val in out.values(): + # Iterate to check for nested "types" like mask + for k, v in type_val.items(): + # Add base data type path + if k == "path": + to_get.append(v) + # Add nested data type path + if isinstance(v, dict) and "path" in v: + to_get.append(v["path"]) if len(to_get) > 0: logger.debug(f"Getting {len(to_get)} files using datalad:") diff --git a/junifer/datagrabber/dmcc13_benchmark.py b/junifer/datagrabber/dmcc13_benchmark.py index e41460c24..af4d32f96 100644 --- a/junifer/datagrabber/dmcc13_benchmark.py +++ b/junifer/datagrabber/dmcc13_benchmark.py @@ -25,17 +25,16 @@ class DMCC13Benchmark(PatternDataladDataGrabber): The directory where the datalad dataset will be cloned. If None, the datalad dataset will be cloned into a temporary directory (default None). - types: {"BOLD", "BOLD_confounds", "BOLD_mask", "T1w", "T1w_mask", \ - "VBM_CSF", "VBM_GM", "VBM_WM", "Warp" (only if \ - "native_t1w = True")} or a list of the options, optional + types: {"BOLD", "T1w", "VBM_CSF", "VBM_GM", "VBM_WM"} or \ + list of the options, optional DMCC data types. If None, all available data types are selected. (default None). - sessions: {"ses-wave1bas", "ses-wave1pro", "ses-wave1rea"} or list of \ - the options, optional + sessions: {"ses-wave1bas", "ses-wave1pro", "ses-wave1rea"} or \ + list of the options, optional DMCC sessions. If None, all available sessions are selected (default None). tasks: {"Rest", "Axcpt", "Cuedts", "Stern", "Stroop"} or \ - list of the options, optional + list of the options, optional DMCC task sessions. If None, all available task sessions are selected (default None). phase_encodings : {"AP", "PA"} or list of the options, optional @@ -148,24 +147,23 @@ class DMCC13Benchmark(PatternDataladDataGrabber): "space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "BOLD_mask", - }, - "BOLD_confounds": { - "pattern": ( - "derivatives/fmriprep-1.3.2/{subject}/{session}/" - "func/{subject}_{session}_task-{task}_acq-mb4" - "{phase_encoding}_run-{run}_desc-confounds_regressors.tsv" - ), - "format": "fmriprep", - }, - "BOLD_mask": { - "pattern": ( - "derivatives/fmriprep-1.3.2/{subject}/{session}/" - "/func/{subject}_{session}_task-{task}_acq-mb4" - "{phase_encoding}_run-{run}_" - "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep-1.3.2/{subject}/{session}/" + "/func/{subject}_{session}_task-{task}_acq-mb4" + "{phase_encoding}_run-{run}_" + "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, + "confounds": { + "pattern": ( + "derivatives/fmriprep-1.3.2/{subject}/{session}/" + "func/{subject}_{session}_task-{task}_acq-mb4" + "{phase_encoding}_run-{run}_desc-confounds_regressors.tsv" + ), + "format": "fmriprep", + }, }, "T1w": { "pattern": ( @@ -173,14 +171,13 @@ class DMCC13Benchmark(PatternDataladDataGrabber): "{subject}_space-MNI152NLin2009cAsym_desc-preproc_T1w.nii.gz" ), "space": "MNI152NLin2009cAsym", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep-1.3.2/{subject}/anat/" - "{subject}_space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" - ), - "space": "MNI152NLin2009cAsym", + "mask": { + "pattern": ( + "derivatives/fmriprep-1.3.2/{subject}/anat/" + "{subject}_space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ), + "space": "MNI152NLin2009cAsym", + }, }, "VBM_CSF": { "pattern": ( @@ -216,14 +213,13 @@ class DMCC13Benchmark(PatternDataladDataGrabber): "{subject}_desc-preproc_T1w.nii.gz" ), "space": "native", - "mask_item": "T1w_mask", - }, - "T1w_mask": { - "pattern": ( - "derivatives/fmriprep-1.3.2/{subject}/anat/" - "{subject}_desc-brain_mask.nii.gz" - ), - "space": "native", + "mask": { + "pattern": ( + "derivatives/fmriprep-1.3.2/{subject}/anat/" + "{subject}_desc-brain_mask.nii.gz" + ), + "space": "native", + }, }, "Warp": { "pattern": ( @@ -298,20 +294,6 @@ class DMCC13Benchmark(PatternDataladDataGrabber): phase_encoding=phase_encoding, run=run, ) - if out.get("BOLD"): - out["BOLD"]["mask_item"] = "BOLD_mask" - # Add space information - out["BOLD"].update({"space": "MNI152NLin2009cAsym"}) - if out.get("T1w"): - out["T1w"]["mask_item"] = "T1w_mask" - # Add space information - if self.native_t1w: - out["T1w"].update({"space": "native"}) - else: - out["T1w"].update({"space": "MNI152NLin2009cAsym"}) - if out.get("Warp"): - # Add source space information - out["Warp"].update({"src": "MNI152NLin2009cAsym"}) return out def get_elements(self) -> List: diff --git a/junifer/datagrabber/pattern.py b/junifer/datagrabber/pattern.py index 41b60b443..395b8f3a1 100644 --- a/junifer/datagrabber/pattern.py +++ b/junifer/datagrabber/pattern.py @@ -40,7 +40,12 @@ class PatternDataGrabber(BaseDataGrabber): { "mandatory": ["pattern", "space"], - "optional": [] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + } } * ``"T2w"`` : @@ -49,7 +54,12 @@ class PatternDataGrabber(BaseDataGrabber): { "mandatory": ["pattern", "space"], - "optional": [] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + } } * ``"BOLD"`` : @@ -58,7 +68,16 @@ class PatternDataGrabber(BaseDataGrabber): { "mandatory": ["pattern", "space"], - "optional": ["mask_item"] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + "confounds": { + "mandatory": ["pattern", "format"], + "optional": [] + } + } } * ``"Warp"`` : @@ -70,15 +89,6 @@ class PatternDataGrabber(BaseDataGrabber): "optional": [] } - * ``"BOLD_confounds"`` : - - .. code-block:: none - - { - "mandatory": ["pattern", "format"], - "optional": [] - } - * ``"VBM_GM"`` : .. code-block:: none @@ -246,6 +256,64 @@ class PatternDataGrabber(BaseDataGrabber): ) return pattern.format(**element) + def _get_path_from_patterns( + self, element: Dict, pattern: str, data_type: str + ) -> Path: + """Get path from resolved patterns. + + Parameters + ---------- + element : dict + The element to be used in the replacement. + pattern : str + The pattern to be replaced. + data_type : str + The data type of the pattern. + + Returns + ------- + pathlib.Path + The path for the resolved pattern. + + Raises + ------ + RuntimeError + If more than one file matches for a data type's pattern or + if no file matches for a data type's pattern or + if file cannot be accessed for an element. + + """ + # Replace element in the pattern for globbing + resolved_pattern = self._replace_patterns_glob(element, pattern) + # Resolve path for wildcard + if "*" in resolved_pattern: + t_matches = list(self.datadir.absolute().glob(resolved_pattern)) + # Multiple matches + if len(t_matches) > 1: + raise_error( + f"More than one file matches for {element} / {data_type}:" + f" {t_matches}", + klass=RuntimeError, + ) + # No matches + elif len(t_matches) == 0: + raise_error( + f"No file matches for {element} / {data_type}", + klass=RuntimeError, + ) + path = t_matches[0] + else: + path = self.datadir / resolved_pattern + if not self.skip_file_check: + if not path.exists() and not path.is_symlink(): + raise_error( + f"Cannot access {data_type} for {element}: " + f"File {path} does not exist", + klass=RuntimeError, + ) + + return path + def get_element_keys(self) -> List[str]: """Get element keys. @@ -279,47 +347,49 @@ class PatternDataGrabber(BaseDataGrabber): Dictionary of dictionaries for each type of data required for the specified element. - Raises - ------ - RuntimeError - If more than one file matches for a data type's pattern or - if no file matches for a data type's pattern or - if file cannot be accessed for an element. - """ out = {} for t_type in self.types: + # Data type dictionary t_pattern = self.patterns[t_type] - t_replace = self._replace_patterns_glob( - element, t_pattern["pattern"] - ) - if "*" in t_replace: - t_matches = list(self.datadir.absolute().glob(t_replace)) - if len(t_matches) > 1: - raise_error( - f"More than one file matches for {element} / {t_type}:" - f" {t_matches}", - klass=RuntimeError, + # Copy data type dictionary in output + out[t_type] = t_pattern.copy() + # Iterate to check for nested "types" like mask + for k, v in t_pattern.items(): + # Resolve pattern for base data type + if k == "pattern": + logger.info(f"Resolving path from pattern for {t_type}") + # Resolve pattern + base_data_type_pattern_path = self._get_path_from_patterns( + element=element, + pattern=v, + data_type=t_type, ) - elif len(t_matches) == 0: - raise_error( - f"No file matches for {element} / {t_type}", - klass=RuntimeError, + # Remove pattern key + out[t_type].pop("pattern") + # Add path key + out[t_type].update({"path": base_data_type_pattern_path}) + # Resolve pattern for nested data type + if isinstance(v, dict) and "pattern" in v: + # Set nested type key for easier access + t_nested_type = f"{t_type}.{k}" + logger.info( + f"Resolving path from pattern for {t_nested_type}" ) - t_out = t_matches[0] - else: - t_out = self.datadir / t_replace - if not self.skip_file_check: - if not t_out.exists() and not t_out.is_symlink(): - raise_error( - f"Cannot access {t_type} for {element}: " - f"File {t_out} does not exist", - klass=RuntimeError, + # Resolve pattern + nested_data_type_pattern_path = ( + self._get_path_from_patterns( + element=element, + pattern=v["pattern"], + data_type=t_nested_type, ) - # Update path for the element - out[t_type] = t_pattern.copy() # copy data type dictionary - out[t_type].pop("pattern") # remove pattern key - out[t_type].update({"path": t_out}) # add path key + ) + # Remove pattern key + out[t_type][k].pop("pattern") + # Add path key + out[t_type][k].update( + {"path": nested_data_type_pattern_path} + ) return out diff --git a/junifer/datagrabber/pattern_datalad.py b/junifer/datagrabber/pattern_datalad.py index 96ed39b48..a8c4eeaa5 100644 --- a/junifer/datagrabber/pattern_datalad.py +++ b/junifer/datagrabber/pattern_datalad.py @@ -32,7 +32,12 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): { "mandatory": ["pattern", "space"], - "optional": [] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + } } * ``"T2w"`` : @@ -41,7 +46,12 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): { "mandatory": ["pattern", "space"], - "optional": [] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + } } * ``"BOLD"`` : @@ -50,7 +60,16 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): { "mandatory": ["pattern", "space"], - "optional": ["mask_item"] + "optional": { + "mask": { + "mandatory": ["pattern", "space"], + "optional": [] + } + "confounds": { + "mandatory": ["pattern", "format"], + "optional": [] + } + } } * ``"Warp"`` : @@ -62,15 +81,6 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): "optional": [] } - * ``"BOLD_confounds"`` : - - .. code-block:: none - - { - "mandatory": ["pattern", "format"], - "optional": [] - } - * ``"VBM_GM"`` : .. code-block:: none diff --git a/junifer/datagrabber/tests/test_datagrabber_utils.py b/junifer/datagrabber/tests/test_datagrabber_utils.py index 0b2ec1ce7..ec5c8e613 100644 --- a/junifer/datagrabber/tests/test_datagrabber_utils.py +++ b/junifer/datagrabber/tests/test_datagrabber_utils.py @@ -151,12 +151,18 @@ def test_validate_replacements( pytest.raises(KeyError, match="Mandatory key"), ), ( - ["BOLD_confounds"], + ["BOLD"], { - "BOLD_confounds": { - "pattern": "{subject}/func/{subject}_confounds.tsv", - "format": "fmriprep", + "BOLD": { + "pattern": ( + "{subject}/func/{subject}_task-rest_bold.nii.gz" + ), "space": "MNINLin6Asym", + "confounds": { + "pattern": "{subject}/func/{subject}_confounds.tsv", + "format": "fmriprep", + }, + "zip": "zap", }, }, pytest.raises(RuntimeError, match="not accepted"), @@ -172,7 +178,7 @@ def test_validate_replacements( pytest.raises(ValueError, match="following a replacement"), ), ( - ["T1w", "T2w", "BOLD", "BOLD_confounds"], + ["T1w", "T2w", "BOLD"], { "T1w": { "pattern": "{subject}/anat/{subject}_T1w.nii.gz", @@ -187,10 +193,10 @@ def test_validate_replacements( "{subject}/func/{subject}_task-rest_bold.nii.gz" ), "space": "MNI152NLin6Asym", - }, - "BOLD_confounds": { - "pattern": "{subject}/func/{subject}_confounds.tsv", - "format": "fmriprep", + "confounds": { + "pattern": "{subject}/func/{subject}_confounds.tsv", + "format": "fmriprep", + }, }, }, nullcontext(), diff --git a/junifer/datagrabber/tests/test_dmcc13_benchmark.py b/junifer/datagrabber/tests/test_dmcc13_benchmark.py index e797ae3f6..2b2566de1 100644 --- a/junifer/datagrabber/tests/test_dmcc13_benchmark.py +++ b/junifer/datagrabber/tests/test_dmcc13_benchmark.py @@ -93,13 +93,10 @@ def test_DMCC13Benchmark( # Available data types data_types = [ "BOLD", - "BOLD_confounds", - "BOLD_mask", "VBM_CSF", "VBM_GM", "VBM_WM", "T1w", - "T1w_mask", ] # Add Warp if native T1w is accessed if native_t1w: @@ -111,14 +108,6 @@ def test_DMCC13Benchmark( f"sub-01_{ses}_task-{task}_acq-mb4{phase}_run-{run}_" "space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz" ), - ( - f"sub-01_{ses}_task-{task}_acq-mb4{phase}_run-{run}_" - "desc-confounds_regressors.tsv" - ), - ( - f"sub-01_{ses}_task-{task}_acq-mb4{phase}_run-{run}_" - "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" - ), "sub-01_space-MNI152NLin2009cAsym_label-CSF_probseg.nii.gz", "sub-01_space-MNI152NLin2009cAsym_label-GM_probseg.nii.gz", "sub-01_space-MNI152NLin2009cAsym_label-WM_probseg.nii.gz", @@ -127,16 +116,12 @@ def test_DMCC13Benchmark( data_file_names.extend( [ "sub-01_desc-preproc_T1w.nii.gz", - "sub-01_desc-brain_mask.nii.gz", "sub-01_from-MNI152NLin2009cAsym_to-T1w_mode-image_xfm.h5", ] ) else: - data_file_names.extend( - [ - "sub-01_space-MNI152NLin2009cAsym_desc-preproc_T1w.nii.gz", - "sub-01_space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz", - ] + data_file_names.append( + "sub-01_space-MNI152NLin2009cAsym_desc-preproc_T1w.nii.gz" ) for data_type, data_file_name in zip(data_types, data_file_names): @@ -151,6 +136,48 @@ def test_DMCC13Benchmark( # Assert metadata assert "meta" in out[data_type] + # Check BOLD nested data types + for type_, file_name in zip( + ("mask", "confounds"), + ( + ( + f"sub-01_{ses}_task-{task}_acq-mb4{phase}_run-{run}_" + "space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ), + ( + f"sub-01_{ses}_task-{task}_acq-mb4{phase}_run-{run}_" + "desc-confounds_regressors.tsv" + ), + ), + ): + # Assert data type + assert type_ in out["BOLD"] + # Assert data file path exists + assert out["BOLD"][type_]["path"].exists() + # Assert data file path is a file + assert out["BOLD"][type_]["path"].is_file() + # Assert data file name + assert out["BOLD"][type_]["path"].name == file_name + + # Check T1w nested data types + # Assert data type + assert "mask" in out["T1w"] + # Assert data file path exists + assert out["T1w"]["mask"]["path"].exists() + # Assert data file path is a file + assert out["T1w"]["mask"]["path"].is_file() + # Assert data file name + if native_t1w: + assert ( + out["T1w"]["mask"]["path"].name + == "sub-01_desc-brain_mask.nii.gz" + ) + else: + assert ( + out["T1w"]["mask"]["path"].name + == "sub-01_space-MNI152NLin2009cAsym_desc-brain_mask.nii.gz" + ) + @pytest.mark.parametrize( "types, native_t1w", @@ -165,8 +192,8 @@ def test_DMCC13Benchmark( ("VBM_GM", False), ("VBM_WM", True), ("VBM_WM", False), - (["BOLD", "BOLD_confounds"], True), - (["BOLD", "BOLD_confounds"], False), + (["BOLD", "VBM_CSF"], True), + (["BOLD", "VBM_CSF"], False), (["T1w", "VBM_CSF"], True), (["T1w", "VBM_CSF"], False), (["VBM_GM", "VBM_WM"], True), diff --git a/junifer/datagrabber/utils.py b/junifer/datagrabber/utils.py index f93564eb4..3a92473a4 100644 --- a/junifer/datagrabber/utils.py +++ b/junifer/datagrabber/utils.py @@ -13,51 +13,45 @@ from ..utils import logger, raise_error PATTERNS_SCHEMA = { "T1w": { "mandatory": ["pattern", "space"], - "optional": ["mask_item"], - }, - "T1w_mask": { - "mandatory": ["pattern", "space"], - "optional": [], + "optional": { + "mask": {"mandatory": ["pattern", "space"], "optional": []}, + }, }, "T2w": { "mandatory": ["pattern", "space"], - "optional": ["mask_item"], - }, - "T2w_mask": { - "mandatory": ["pattern", "space"], - "optional": [], + "optional": { + "mask": {"mandatory": ["pattern", "space"], "optional": []}, + }, }, "BOLD": { "mandatory": ["pattern", "space"], - "optional": ["mask_item"], - }, - "BOLD_confounds": { - "mandatory": ["pattern", "format"], - "optional": [], - }, - "BOLD_mask": { - "mandatory": ["pattern", "space"], - "optional": [], + "optional": { + "mask": {"mandatory": ["pattern", "space"], "optional": []}, + "confounds": { + "mandatory": ["pattern", "format"], + "optional": ["mappings"], + }, + }, }, "Warp": { "mandatory": ["pattern", "src", "dst"], - "optional": [], + "optional": {}, }, "VBM_GM": { "mandatory": ["pattern", "space"], - "optional": [], + "optional": {}, }, "VBM_WM": { "mandatory": ["pattern", "space"], - "optional": [], + "optional": {}, }, "VBM_CSF": { "mandatory": ["pattern", "space"], - "optional": [], + "optional": {}, }, "DWI": { "mandatory": ["pattern"], - "optional": [], + "optional": {}, }, } @@ -129,6 +123,67 @@ def validate_replacements( raise_error(msg="At least one pattern must contain all replacements.") +def _validate_mandatory_keys( + keys: List[str], schema: List[str], data_type: str +) -> None: + """Validate mandatory keys. + + Parameters + ---------- + keys : list of str + The keys to validate. + schema : list of str + The schema to validate against. + data_type : str + The data type being validated. + + Raises + ------ + KeyError + If any mandatory key is missing for a data type. + + """ + for key in schema: + if key not in keys: + raise_error( + msg=f"Mandatory key: `{key}` missing for {data_type}", + klass=KeyError, + ) + else: + logger.debug(f"Mandatory key: `{key}` found for {data_type}") + + +def _identify_stray_keys( + keys: List[str], schema: List[str], data_type: str +) -> None: + """Identify stray keys. + + Parameters + ---------- + keys : list of str + The keys to check. + schema : list of str + The schema to check against. + data_type : str + The data type being checked. + + Raises + ------ + RuntimeError + If an unknown key is found for a data type. + + """ + for key in keys: + if key not in schema: + raise_error( + msg=( + f"Key: {key} not accepted for {data_type} " + "pattern, remove it to proceed" + ), + klass=RuntimeError, + ) + + def validate_patterns( types: List[str], patterns: Dict[str, Dict[str, str]] ) -> None: @@ -143,10 +198,6 @@ def validate_patterns( Raises ------ - KeyError - If any mandatory key is missing for a data type. - RuntimeError - If an unknown key is found for a data type. TypeError If ``patterns`` is not a dictionary. ValueError @@ -180,22 +231,15 @@ def validate_patterns( f"should be one of: {list(PATTERNS_SCHEMA.keys())}" ) # Check mandatory keys for data type - for mandatory_key in PATTERNS_SCHEMA[data_type_key]["mandatory"]: - if mandatory_key not in data_type_val: - raise_error( - msg=( - f"Mandatory key: `{mandatory_key}` missing for " - f"{data_type_key}" - ), - klass=KeyError, - ) - else: - logger.debug( - f"Mandatory key: `{mandatory_key}` found for " - f"{data_type_key}" - ) + _validate_mandatory_keys( + keys=list(data_type_val), + schema=PATTERNS_SCHEMA[data_type_key]["mandatory"], + data_type=data_type_key, + ) # Check optional keys for data type - for optional_key in PATTERNS_SCHEMA[data_type_key]["optional"]: + for optional_key, optional_val in PATTERNS_SCHEMA[data_type_key][ + "optional" + ].items(): if optional_key not in data_type_val: logger.debug( f"Optional key: `{optional_key}` missing for " @@ -206,19 +250,48 @@ def validate_patterns( f"Optional key: `{optional_key}` found for " f"{data_type_key}" ) - # Check stray key for data type - for key in data_type_val.keys(): - if key not in ( - PATTERNS_SCHEMA[data_type_key]["mandatory"] - + PATTERNS_SCHEMA[data_type_key]["optional"] - ): - raise_error( - msg=( - f"Key: {key} not accepted for {data_type_key} " - "pattern, remove it to proceed" - ), - klass=RuntimeError, + # Set nested type name for easier access + nested_data_type = f"{data_type_key}.{optional_key}" + nested_mandatory_keys_schema = PATTERNS_SCHEMA[data_type_key][ + "optional" + ][optional_key]["mandatory"] + nested_optional_keys_schema = PATTERNS_SCHEMA[data_type_key][ + "optional" + ][optional_key]["optional"] + # Check mandatory keys for nested type + _validate_mandatory_keys( + keys=list(optional_val["mandatory"]), + schema=nested_mandatory_keys_schema, + data_type=nested_data_type, ) + # Check optional keys for nested type + for nested_optional_key in nested_optional_keys_schema: + if nested_optional_key not in optional_val["optional"]: + logger.debug( + f"Optional key: `{nested_optional_key}` missing " + f"for {nested_data_type}" + ) + else: + logger.debug( + f"Optional key: `{nested_optional_key}` found for " + f"{nested_data_type}" + ) + # Check stray key for nested data type + _identify_stray_keys( + keys=optional_val["mandatory"] + optional_val["optional"], + schema=nested_mandatory_keys_schema + + nested_optional_keys_schema, + data_type=nested_data_type, + ) + # Check stray key for data type + _identify_stray_keys( + keys=list(data_type_val.keys()), + schema=( + PATTERNS_SCHEMA[data_type_key]["mandatory"] + + list(PATTERNS_SCHEMA[data_type_key]["optional"].keys()) + ), + data_type=data_type_key, + ) # Wildcard check in patterns if "}*" in data_type_val["pattern"]: raise_error( diff --git a/junifer/datareader/default.py b/junifer/datareader/default.py index 0a9a77ad7..438e13ee0 100644 --- a/junifer/datareader/default.py +++ b/junifer/datareader/default.py @@ -5,7 +5,7 @@ # License: AGPL from pathlib import Path -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union import nibabel as nib import pandas as pd @@ -102,57 +102,106 @@ class DefaultDataReader(PipelineStepMixin, UpdateMetaMixin): if params is None: params = {} # For each type of data, try to read it - for type_ in input.keys(): + for type_key, type_val in input.items(): # Skip Warp data type - if type_ == "Warp": + if type_key == "Warp": continue # Check for malformed datagrabber specification - if "path" not in input[type_]: + if "path" not in type_val: warn_with_log( - f"Input type {type_} does not provide a path. Skipping." + f"Input type {type_key} does not provide a path. Skipping." ) continue - # Retrieve actual path - t_path = input[type_]["path"] - # Retrieve loading params for the data type - t_params = params.get(type_, {}) - - # Convert str to Path - if not isinstance(t_path, Path): - t_path = Path(t_path) - out[type_]["path"] = t_path - - logger.info(f"Reading {type_} from {t_path.as_posix()}") - # Initialize variable for file data - fread = None - # Lowercase path - fname = t_path.name.lower() - # Loop through extensions to find the correct one - for ext, ftype in _extensions.items(): - if fname.endswith(ext): - logger.info(f"{type_} is type {ftype}") - # Retrieve reader function - reader_func = _readers[ftype]["func"] - # Retrieve reader function params - reader_params = _readers[ftype]["params"] - # Update reader function params - if reader_params is not None: - t_params.update(reader_params) - logger.debug(f"Calling {reader_func} with {t_params}") + # Iterate to check for nested "types" like mask; + # need to copy to avoid runtime error for changing dict size + for k, v in type_val.copy().items(): + # Read data for base data type + if k == "path": + # Convert str to Path + if not isinstance(v, Path): + v = Path(v) + # Update path + out[type_key]["path"] = v + logger.info(f"Reading {type_key} from {v.absolute()!s}") + # Retrieve loading params for the data type + t_params = params.get(type_key, {}) # Read data - fread = reader_func(t_path, **t_params) - break - # If no file data is found due to unknown extension - if fread is None: - logger.info( - f"Unknown file type {t_path.as_posix()}, skipping reading" - ) + out[type_key]["data"] = _read_data( + data_type=type_key, path=v, read_params=t_params + ) + # Read data for nested data type + if isinstance(v, dict) and "path" in v: + # Set path + nested_path = v["path"] + # Convert str to Path + if not isinstance(nested_path, Path): + nested_path = Path(nested_path) + # Update path + out[type_key][k]["path"] = nested_path + # Set nested type key for easier access + nested_type = f"{type_key}.{k}" + logger.info( + f"Reading {nested_type} from " + f"{nested_path.absolute()!s}" + ) + # Retrieve loading params for the nested data type + nested_params = params.get(nested_type, {}) + # Read data + out[type_key][k]["data"] = _read_data( + data_type=nested_type, + path=nested_path, + read_params=nested_params, + ) - # Set file data for output - out[type_]["data"] = fread # Update metadata for step - self.update_meta(out[type_], "datareader") + self.update_meta(out[type_key], "datareader") return out + + +def _read_data( + data_type: str, path: Path, read_params: Dict +) -> Union[nib.Nifti1Image, pd.DataFrame, None]: + """Read data for data type. + + Parameters + ---------- + data_type : str + The data type being read. + path : pathlib.Path + The path to read data from. + read_params : dict + Parameters for reader function. + + Returns + ------- + nibabel.Nifti1Image or pandas.DataFrame or pandas.TextFileReader or None + The data loaded in memory if file type is known else None. + + """ + # Initialize variable for file data + fread = None + # Lowercase path + fname = path.name.lower() + # Loop through extensions to find the correct one + for ext, ftype in _extensions.items(): + if fname.endswith(ext): + logger.info(f"{data_type} is of type {ftype}") + # Retrieve reader function + reader_func = _readers[ftype]["func"] + # Retrieve reader function params + reader_params = _readers[ftype]["params"] + # Update reader function params + if reader_params is not None: + read_params.update(reader_params) + logger.debug(f"Calling {reader_func!s} with {read_params}") + # Read data + fread = reader_func(path, **read_params) + break + # If no file data is found due to unknown extension + if fread is None: + logger.info(f"Unknown file type {path.absolute()!s}, skipping reading") + + return fread diff --git a/junifer/preprocess/base.py b/junifer/preprocess/base.py index d29b5dc40..b46fe7afe 100644 --- a/junifer/preprocess/base.py +++ b/junifer/preprocess/base.py @@ -146,8 +146,8 @@ class BasePreprocessor(ABC, PipelineStepMixin, UpdateMetaMixin): The computed result as dictionary. dict or None Extra "helper" data types as dictionary to add to the Junifer Data - object. For example, computed BOLD mask can be passed via this. - If no new "helper" data types is created, None is to be passed. + object. If no new "helper" data type(s) is(are) created, None is to + be passed. """ raise_error( diff --git a/junifer/preprocess/confounds/fmriprep_confound_remover.py b/junifer/preprocess/confounds/fmriprep_confound_remover.py index 153667a8f..4e77a8e1b 100644 --- a/junifer/preprocess/confounds/fmriprep_confound_remover.py +++ b/junifer/preprocess/confounds/fmriprep_confound_remover.py @@ -203,9 +203,7 @@ class fMRIPrepConfoundRemover(BasePreprocessor): "include it in the future", klass=ValueError, ) - super().__init__( - on="BOLD", required_data_types=["BOLD", "BOLD_confounds"] - ) + super().__init__(on="BOLD", required_data_types=["BOLD"]) def get_valid_inputs(self) -> List[str]: """Get valid data types for input. @@ -361,7 +359,7 @@ class fMRIPrepConfoundRemover(BasePreprocessor): Parameters ---------- input : dict - Dictionary containing the ``BOLD_confounds`` value from the + Dictionary containing the ``BOLD.confounds`` value from the Junifer Data object. Returns @@ -370,7 +368,6 @@ class fMRIPrepConfoundRemover(BasePreprocessor): Dataframe containing the relevant confounds. """ - confounds_format = input["format"] if confounds_format == "adhoc": self._map_adhoc_to_fmriprep(input) @@ -416,50 +413,42 @@ class fMRIPrepConfoundRemover(BasePreprocessor): def _validate_data( self, input: Dict[str, Any], - extra_input: Optional[Dict[str, Any]] = None, ) -> None: """Validate input data. Parameters ---------- input : dict - Dictionary containing the ``BOLD`` value from the + Dictionary containing the ``BOLD`` data from the Junifer Data object. - extra_input : dict, optional - Dictionary containing the rest of the Junifer Data object. Must - include the ``BOLD_confounds`` key. Raises ------ ValueError - If ``extra_input`` is None or - if ``"BOLD_confounds"`` is not found in ``extra_input`` or - if ``"data"`` key is not found in ``"BOLD_confounds"`` or - if ``"data"`` is not pandas.DataFrame or + If ``"confounds"`` is not found in ``input`` or + if ``"data"`` key is not found in ``"input.confounds"`` or + if ``"input.confounds.data"`` is not pandas.DataFrame or if image time series and confounds have different lengths or - if ``"format"`` is not found in ``"BOLD_confounds"`` or - if ``format = "adhoc"`` and ``"mappings"`` key or ``"fmriprep"`` - key or correct fMRIPrep mappings or required fMRIPrep mappings are - not found or if invalid confounds format is found. + if ``format = "adhoc"`` and ``"mappings"`` key is not found or + ``"fmriprep"`` key is not found in ``"mappings"`` or + ``"fmriprep"`` has incorrect fMRIPrep mappings or required + fMRIPrep mappings are not found or + if invalid confounds format is found. """ # BOLD must be 4D niimg check_niimg_4d(input["data"]) - # Check for extra inputs - if extra_input is None: - raise_error( - "No extra input provided, requires `BOLD_confounds` data type " - "in particular" - ) - if "BOLD_confounds" not in extra_input: - raise_error("`BOLD_confounds` data type not provided") - if "data" not in extra_input["BOLD_confounds"]: - raise_error("`BOLD_confounds.data` not provided") - # Confounds must be a pandas.DataFrame - if not isinstance(extra_input["BOLD_confounds"]["data"], pd.DataFrame): - raise_error("`BOLD_confounds.data` must be a `pandas.DataFrame`") + # Check for confound data + if "confounds" not in input: + raise_error("`BOLD.confounds` data type not provided") + if "data" not in input["confounds"]: + raise_error("`BOLD.confounds.data` not provided") + # Confounds must be a pandas.DataFrame; + # if extension is unknown, will not be read, which will give None + if not isinstance(input["confounds"]["data"], pd.DataFrame): + raise_error("`BOLD.confounds.data` must be a `pandas.DataFrame`") - confound_df = extra_input["BOLD_confounds"]["data"] + confound_df = input["confounds"]["data"] bold_img = input["data"] if bold_img.get_fdata().shape[3] != len(confound_df): raise_error( @@ -469,23 +458,19 @@ class fMRIPrepConfoundRemover(BasePreprocessor): ) # Check format - if "format" not in extra_input["BOLD_confounds"]: - raise_error("`BOLD_confounds.format` not provided") - t_format = extra_input["BOLD_confounds"]["format"] + t_format = input["confounds"]["format"] if t_format == "adhoc": - if "mappings" not in extra_input["BOLD_confounds"]: + if "mappings" not in input["confounds"]: raise_error( - "`BOLD_confounds.mappings` need to be set when " - "`BOLD_confounds.format == 'adhoc'`" + "`BOLD.confounds.mappings` need to be set when " + "`BOLD.confounds.format == 'adhoc'`" ) - if "fmriprep" not in extra_input["BOLD_confounds"]["mappings"]: + if "fmriprep" not in input["confounds"]["mappings"]: raise_error( - "`BOLD_confounds.mappings.fmriprep` need to be set when " - "`BOLD_confounds.format == 'adhoc'`" + "`BOLD.confounds.mappings.fmriprep` need to be set when " + "`BOLD.confounds.format == 'adhoc'`" ) - fmriprep_mappings = extra_input["BOLD_confounds"]["mappings"][ - "fmriprep" - ] + fmriprep_mappings = input["confounds"]["mappings"]["fmriprep"] wrong_names = [ x for x in fmriprep_mappings.values() @@ -525,22 +510,22 @@ class fMRIPrepConfoundRemover(BasePreprocessor): input : dict A single input from the Junifer Data object to preprocess. extra_input : dict, optional - The other fields in the Junifer Data object. Must include the - ``BOLD_confounds`` key. + The other fields in the Junifer Data object. Returns ------- dict - The computed result as dictionary. - dict or None - If `self.masks` is not None, then the target data computed mask is - returned else None. + The computed result as dictionary. If `self.masks` is not None, + then the target data computed mask is updated for further steps. + None + Extra "helper" data types as dictionary to add to the Junifer Data + object. """ # Validate data - self._validate_data(input, extra_input) + self._validate_data(input) # Pick confounds - confounds_df = self._pick_confounds(extra_input["BOLD_confounds"]) # type: ignore + confounds_df = self._pick_confounds(input["confounds"]) # type: ignore # Get BOLD data bold_img = input["data"] # Set t_r @@ -553,7 +538,6 @@ class fMRIPrepConfoundRemover(BasePreprocessor): ) # Set mask data mask_img = None - bold_mask_dict = None if self.masks is not None: logger.debug(f"Masking with {self.masks}") mask_img = get_mask( @@ -561,15 +545,15 @@ class fMRIPrepConfoundRemover(BasePreprocessor): ) # Return the BOLD mask and link it to the BOLD data type dict; # this allows to use "inherit" down the pipeline - if extra_input is not None: - logger.debug("Setting `BOLD.mask_item`") - input["mask_item"] = "BOLD_mask" - bold_mask_dict = { - "BOLD_mask": { + logger.debug("Setting `BOLD.mask`") + input.update( + { + "mask": { "data": mask_img, "space": input["space"], } } + ) # Clean image logger.info("Cleaning image using nilearn") logger.debug(f"\tdetrend: {self.detrend}") @@ -587,4 +571,4 @@ class fMRIPrepConfoundRemover(BasePreprocessor): mask_img=mask_img, ) - return input, bold_mask_dict + return input, None diff --git a/junifer/preprocess/confounds/tests/test_fmriprep_confound_remover.py b/junifer/preprocess/confounds/tests/test_fmriprep_confound_remover.py index cb13452ba..cb4ae9586 100644 --- a/junifer/preprocess/confounds/tests/test_fmriprep_confound_remover.py +++ b/junifer/preprocess/confounds/tests/test_fmriprep_confound_remover.py @@ -20,6 +20,7 @@ from junifer.testing import get_testing_data from junifer.testing.datagrabbers import ( OasisVBMTestingDataGrabber, PartlyCloudyTestingDataGrabber, + SPMAuditoryTestingDataGrabber, ) @@ -42,35 +43,10 @@ def test_fMRIPrepConfoundRemover_init() -> None: @pytest.mark.parametrize( "input_", [ - ["T1w"], ["BOLD"], ["T1w", "BOLD"], ], ) -def test_fMRIPrepConfoundRemover_validate_input_errors( - input_: List[str], -) -> None: - """Test errors for fMRIPrepConfoundRemover validate_input. - - Parameters - ---------- - input_ : list of str - The input data types. - - """ - confound_remover = fMRIPrepConfoundRemover() - - with pytest.raises(ValueError, match="not have the required data"): - confound_remover.validate_input(input_) - - -@pytest.mark.parametrize( - "input_", - [ - ["BOLD", "BOLD_confounds"], - ["T1w", "BOLD", "BOLD_confounds"], - ], -) def test_fMRIPrepConfoundRemover_validate_input(input_: List[str]) -> None: """Test fMRIPrepConfoundRemover validate_input. @@ -302,13 +278,13 @@ def test_fMRIPRepConfoundRemover__pick_confounds_fmriprep() -> None: with PartlyCloudyTestingDataGrabber() as dg: input = dg["sub-01"] input = reader.fit_transform(input) - out1 = confound_remover._pick_confounds(input["BOLD_confounds"]) + out1 = confound_remover._pick_confounds(input["BOLD"]["confounds"]) assert set(out1.columns) == {*fmriprep_all_vars, "spike"} with PartlyCloudyTestingDataGrabber(reduce_confounds=False) as dg: input = dg["sub-01"] input = reader.fit_transform(input) - out2 = confound_remover._pick_confounds(input["BOLD_confounds"]) + out2 = confound_remover._pick_confounds(input["BOLD"]["confounds"]) assert set(out2.columns) == {*fmriprep_all_vars, "spike"} assert_frame_equal(out1, out2) @@ -348,123 +324,106 @@ def test_fMRIPRepConfoundRemover__pick_confounds_fmriprep_compute() -> None: def test_fMRIPrepConfoundRemover__validate_data() -> None: """Test fMRIPrepConfoundRemover validate data.""" confound_remover = fMRIPrepConfoundRemover(strategy={"wm_csf": "full"}) - + # Check correct data type with OasisVBMTestingDataGrabber() as dg: element_data = DefaultDataReader().fit_transform(dg["sub-01"]) vbm = element_data["VBM_GM"] with pytest.raises( DimensionError, match="incompatible dimensionality" ): - confound_remover._validate_data(vbm, None) - - with PartlyCloudyTestingDataGrabber(reduce_confounds=False) as dg: + confound_remover._validate_data(vbm) + # Check missing nested type in correct data type + with SPMAuditoryTestingDataGrabber() as dg: element_data = DefaultDataReader().fit_transform(dg["sub-01"]) bold = element_data["BOLD"] - - with pytest.raises(ValueError, match="No extra input"): - confound_remover._validate_data(bold, None) + # Test confound type with pytest.raises( - ValueError, match="`BOLD_confounds` data type not provided" + ValueError, match="`BOLD.confounds` data type not provided" ): - confound_remover._validate_data(bold, {}) + confound_remover._validate_data(bold) + # Test confound data + bold["confounds"] = {} with pytest.raises( - ValueError, match="`BOLD_confounds.data` not provided" + ValueError, match="`BOLD.confounds.data` not provided" ): - confound_remover._validate_data(bold, {"BOLD_confounds": {}}) - - extra_input = { - "BOLD_confounds": {"data": "wrong"}, - } - msg = "must be a `pandas.DataFrame`" - with pytest.raises(ValueError, match=msg): - confound_remover._validate_data(bold, extra_input) - - extra_input = {"BOLD_confounds": {"data": pd.DataFrame()}} + confound_remover._validate_data(bold) + # Test confound data is valid type + bold["confounds"] = {"data": None} + with pytest.raises(ValueError, match="must be a `pandas.DataFrame`"): + confound_remover._validate_data(bold) + # Test confound data dimension mismatch with BOLD + bold["confounds"] = {"data": pd.DataFrame()} with pytest.raises(ValueError, match="Image time series and"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": {"data": element_data["BOLD_confounds"]["data"]} - } - with pytest.raises( - ValueError, match="`BOLD_confounds.format` not provided" - ): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "wrong", - } + confound_remover._validate_data(bold) + # Check nested type variations + with PartlyCloudyTestingDataGrabber(reduce_confounds=False) as dg: + element_data = DefaultDataReader().fit_transform(dg["sub-01"]) + # Test format + modified_bold = { + "data": element_data["BOLD"]["data"], + "confounds": { + "data": element_data["BOLD"]["confounds"]["data"], + "format": "adhoc", + }, } + # Test incorrect format + modified_bold["confounds"].update({"format": "wrong"}) with pytest.raises(ValueError, match="Invalid confounds format"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "adhoc", - } - } - with pytest.raises(ValueError, match="need to be set"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "adhoc", - "mappings": {}, - } - } - with pytest.raises(ValueError, match="need to be set"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "adhoc", + confound_remover._validate_data(modified_bold) + # Test missing mappings for adhoc + modified_bold["confounds"].update({"format": "adhoc"}) + with pytest.raises( + ValueError, match="`BOLD.confounds.mappings` need to be set" + ): + confound_remover._validate_data(modified_bold) + # Test missing fmriprep mappings for adhoc + modified_bold["confounds"].update({"mappings": {}}) + with pytest.raises( + ValueError, + match="`BOLD.confounds.mappings.fmriprep` need to be set", + ): + confound_remover._validate_data(modified_bold) + # Test incorrect fmriprep mappings for adhoc + modified_bold["confounds"].update( + { "mappings": { "fmriprep": { "rot_x": "wrong", "rot_y": "rot_z", "rot_z": "rot_y", - } - }, + }, + } } - } + ) with pytest.raises(ValueError, match=r"names: \['wrong'\]"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "adhoc", + confound_remover._validate_data(modified_bold) + # Test missing fmriprep mappings for adhoc + modified_bold["confounds"].update( + { "mappings": { "fmriprep": { "wrong": "rot_x", "rot_y": "rot_z", "rot_z": "rot_y", - } - }, + }, + } } - } + ) with pytest.raises(ValueError, match=r"Missing columns: \['wrong'\]"): - confound_remover._validate_data(bold, extra_input) - - extra_input = { - "BOLD_confounds": { - "data": element_data["BOLD_confounds"]["data"], - "format": "adhoc", + confound_remover._validate_data(modified_bold) + # Test correct adhoc format + modified_bold["confounds"].update( + { "mappings": { "fmriprep": { "rot_x": "rot_x", "rot_y": "rot_z", "rot_z": "rot_y", - } - }, + }, + } } - } - confound_remover._validate_data(bold, extra_input) + ) + confound_remover._validate_data(modified_bold) def test_fMRIPrepConfoundRemover_preprocess() -> None: @@ -476,7 +435,9 @@ def test_fMRIPrepConfoundRemover_preprocess() -> None: element_data = DefaultDataReader().fit_transform(dg["sub-01"]) orig_bold = element_data["BOLD"]["data"].get_fdata().copy() pre_input = element_data["BOLD"] - pre_extra_input = {"BOLD_confounds": element_data["BOLD_confounds"]} + pre_extra_input = { + "BOLD": {"confounds": element_data["BOLD"]["confounds"]} + } output, _ = confound_remover.preprocess(pre_input, pre_extra_input) trans_bold = output["data"].get_fdata() # Transformation is in place @@ -530,7 +491,7 @@ def test_fMRIPrepConfoundRemover_fit_transform() -> None: assert t_meta["t_r"] is None assert t_meta["masks"] is None - assert "BOLD_mask" not in output + assert "mask" not in output["BOLD"] assert "dependencies" in output["BOLD"]["meta"] dependencies = output["BOLD"]["meta"]["dependencies"] @@ -582,9 +543,7 @@ def test_fMRIPrepConfoundRemover_fit_transform_masks() -> None: assert "threshold" in t_meta["masks"]["compute_brain_mask"] assert t_meta["masks"]["compute_brain_mask"]["threshold"] == 0.2 - assert "BOLD_mask" in output - assert "mask_item" in output["BOLD"] - assert output["BOLD"]["mask_item"] == "BOLD_mask" + assert "mask" in output["BOLD"] assert "dependencies" in output["BOLD"]["meta"] dependencies = output["BOLD"]["meta"]["dependencies"] diff --git a/junifer/testing/datagrabbers.py b/junifer/testing/datagrabbers.py index f14a11fdb..c59fb3634 100644 --- a/junifer/testing/datagrabbers.py +++ b/junifer/testing/datagrabbers.py @@ -181,7 +181,7 @@ class PartlyCloudyTestingDataGrabber(BaseDataGrabber): """Initialize the class.""" datadir = tempfile.mkdtemp() # Define types - types = ["BOLD", "BOLD_confounds"] + types = ["BOLD"] self.reduce_confounds = reduce_confounds self.age_group = age_group super().__init__(types=types, datadir=datadir) @@ -242,10 +242,10 @@ class PartlyCloudyTestingDataGrabber(BaseDataGrabber): out["BOLD"] = { "path": Path(self._dataset["func"][i_sub]), "space": "MNI152NLin2009cAsym", - } - out["BOLD_confounds"] = { - "path": Path(self._dataset["confounds"][i_sub]), - "format": "fmriprep", + "confounds": { + "path": Path(self._dataset["confounds"][i_sub]), + "format": "fmriprep", + }, } return out diff --git a/junifer/testing/tests/test_partlycloudytesting_datagrabber.py b/junifer/testing/tests/test_partlycloudytesting_datagrabber.py index 5d228644a..9c9a8c92b 100644 --- a/junifer/testing/tests/test_partlycloudytesting_datagrabber.py +++ b/junifer/testing/tests/test_partlycloudytesting_datagrabber.py @@ -28,13 +28,13 @@ def test_PartlyCloudyTestingDataGrabber() -> None: assert out["BOLD"]["path"].exists() assert out["BOLD"]["path"].is_file() - assert "BOLD_confounds" in out - assert out["BOLD_confounds"]["path"].exists() - assert out["BOLD_confounds"]["path"].is_file() - assert "format" in out["BOLD_confounds"] - assert "fmriprep" == out["BOLD_confounds"]["format"] + assert "confounds" in out["BOLD"] + assert out["BOLD"]["confounds"]["path"].exists() + assert out["BOLD"]["confounds"]["path"].is_file() + assert "format" in out["BOLD"]["confounds"] + assert "fmriprep" == out["BOLD"]["confounds"]["format"] with PartlyCloudyTestingDataGrabber(reduce_confounds=False) as dg: out = dg["sub-01"] - assert "format" in out["BOLD_confounds"] - assert "fmriprep" == out["BOLD_confounds"]["format"] + assert "format" in out["BOLD"]["confounds"] + assert "fmriprep" == out["BOLD"]["confounds"]["format"]