From fc74e2b42acf9e91dd4de722de18840bc32d5eb0 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 30 Mar 2023 15:45:54 +0200 Subject: [PATCH 1/5] add ICA+FIx param --- junifer/datagrabber/hcp.py | 21 +++++++++++++++++++-- tools/create_hcp1200_example_dataset.py | 15 +++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index d1048c034..e5a35053c 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -26,9 +26,12 @@ class HCP1200(PatternDataGrabber): phase_encodings : {"LR", "RL"} or list of the options, optional HCP phase encoding directions. If None, both will be used (default None). + icafix : bool, optional + Whether to use retrieve data that was processed with ICA+FIX. + Only 'REST1' and 'REST2' tasks are available with ICA+FIX (default + False). **kwargs Keyword arguments passed to superclass. - """ def __init__( @@ -36,6 +39,7 @@ class HCP1200(PatternDataGrabber): datadir: Union[str, Path], tasks: Union[str, List[str], None] = None, phase_encodings: Union[str, List[str], None] = None, + ica_fix: bool = False, **kwargs, ) -> None: # All tasks @@ -82,6 +86,12 @@ class HCP1200(PatternDataGrabber): f"{all_phase_encodings}." ) + if ica_fix is True: + if not all([task in ["REST1", "REST2"] for task in self.tasks]): + raise_error( + "ICA+FIX is only available for 'REST1' and 'REST2' tasks." + ) + suffix = "_hp2000_clean" if ica_fix is True else "" # The types of data types = ["BOLD"] # The patterns @@ -89,7 +99,8 @@ class HCP1200(PatternDataGrabber): "BOLD": ( "{subject}/MNINonLinear/Results/" "{task}_{phase_encoding}/" - "{task}_{phase_encoding}_hp2000_clean.nii.gz" + "{task}_{phase_encoding}" + f"{suffix}.nii.gz" ) } # The replacements @@ -173,6 +184,10 @@ class DataladHCP1200(DataladDataGrabber, HCP1200): phase_encodings : {"LR", "RL"} or list of the options, optional HCP phase encoding directions. If None, both will be used (default None). + icafix : bool, optional + Whether to use retrieve data that was processed with ICA+FIX. + Only 'REST1' and 'REST2' tasks are available with ICA+FIX (default + False). """ def __init__( @@ -180,6 +195,7 @@ class DataladHCP1200(DataladDataGrabber, HCP1200): datadir: Union[str, Path, None] = None, tasks: Union[str, List[str], None] = None, phase_encodings: Union[str, List[str], None] = None, + ica_fix: bool = False, ) -> None: uri = ( "https://github.com/datalad-datasets/" @@ -192,6 +208,7 @@ class DataladHCP1200(DataladDataGrabber, HCP1200): phase_encodings=phase_encodings, uri=uri, rootdir=rootdir, + ica_fix=ica_fix, ) @property diff --git a/tools/create_hcp1200_example_dataset.py b/tools/create_hcp1200_example_dataset.py index 4aa4044ea..5a3bd0637 100644 --- a/tools/create_hcp1200_example_dataset.py +++ b/tools/create_hcp1200_example_dataset.py @@ -22,7 +22,7 @@ if __name__ == "__main__": # Set base directory basedir = tmpdir_path / "example_hcp1200" # Create new datalad dataset - dataset = dl.create(path=str(basedir.absolute())) + dataset = dl.create(path=str(basedir.absolute())) # type: ignore # Generate subject directories for sub in range(1, 10): subdir = basedir / f"sub-{sub:02d}" @@ -57,15 +57,26 @@ if __name__ == "__main__": ) # Create subject data directory sub_datadir.mkdir(parents=True) + # Set subject data file sub_datafile = ( sub_datadir - / f"{new_task}_{phase_encoding}_hp2000_clean.nii.gz" + / f"{new_task}_{phase_encoding}.nii.gz" ) # Create subject data file with open(sub_datafile, "w") as f: f.write("placeholder") + if "REST" in task: + # Set subject data file with ICA+FIX + sub_datafile = ( + sub_datadir + / f"{new_task}_{phase_encoding}_hp2000_clean.nii.gz" + ) + # Create subject data file with ICA+FIX + with open(sub_datafile, "w") as f: + f.write("placeholder") + # Save datalad dataset dataset.save(recursive=True) # Add datalad sibling -- 2.52.0 From 244f87f4a632d7129ad7f6ad9fbb96b81f77534d Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 30 Mar 2023 15:52:47 +0200 Subject: [PATCH 2/5] Add tests --- junifer/datagrabber/tests/test_hcp.py | 83 ++++++++++++++++++++------- 1 file changed, 63 insertions(+), 20 deletions(-) diff --git a/junifer/datagrabber/tests/test_hcp.py b/junifer/datagrabber/tests/test_hcp.py index 375f4409d..c5ca8ec5b 100644 --- a/junifer/datagrabber/tests/test_hcp.py +++ b/junifer/datagrabber/tests/test_hcp.py @@ -29,33 +29,38 @@ def hcpdg() -> Iterable[DataladHCP1200]: @pytest.mark.parametrize( - "tasks, phase_encodings, expected_path_name", + "tasks, phase_encodings, ica_fix, expected_path_name", [ - (None, None, "rfMRI_REST1_LR_hp2000_clean.nii.gz"), - ("REST1", "LR", "rfMRI_REST1_LR_hp2000_clean.nii.gz"), - ("REST1", "RL", "rfMRI_REST1_RL_hp2000_clean.nii.gz"), - ("REST2", "LR", "rfMRI_REST2_LR_hp2000_clean.nii.gz"), - ("REST2", "RL", "rfMRI_REST2_RL_hp2000_clean.nii.gz"), - ("SOCIAL", "LR", "tfMRI_SOCIAL_LR_hp2000_clean.nii.gz"), - ("SOCIAL", "RL", "tfMRI_SOCIAL_RL_hp2000_clean.nii.gz"), - ("WM", "LR", "tfMRI_WM_LR_hp2000_clean.nii.gz"), - ("WM", "RL", "tfMRI_WM_RL_hp2000_clean.nii.gz"), - ("RELATIONAL", "LR", "tfMRI_RELATIONAL_LR_hp2000_clean.nii.gz"), - ("RELATIONAL", "RL", "tfMRI_RELATIONAL_RL_hp2000_clean.nii.gz"), - ("EMOTION", "LR", "tfMRI_EMOTION_LR_hp2000_clean.nii.gz"), - ("EMOTION", "RL", "tfMRI_EMOTION_RL_hp2000_clean.nii.gz"), - ("LANGUAGE", "LR", "tfMRI_LANGUAGE_LR_hp2000_clean.nii.gz"), - ("LANGUAGE", "RL", "tfMRI_LANGUAGE_RL_hp2000_clean.nii.gz"), - ("GAMBLING", "LR", "tfMRI_GAMBLING_LR_hp2000_clean.nii.gz"), - ("GAMBLING", "RL", "tfMRI_GAMBLING_RL_hp2000_clean.nii.gz"), - ("MOTOR", "LR", "tfMRI_MOTOR_LR_hp2000_clean.nii.gz"), - ("MOTOR", "RL", "tfMRI_MOTOR_RL_hp2000_clean.nii.gz"), + (None, None, False, "rfMRI_REST1_LR.nii.gz"), + ("REST1", "LR", False, "rfMRI_REST1_LR.nii.gz"), + ("REST1", "RL", False, "rfMRI_REST1_RL.nii.gz"), + ("REST2", "LR", False, "rfMRI_REST2_LR.nii.gz"), + ("REST2", "RL", False, "rfMRI_REST2_RL.nii.gz"), + ("SOCIAL", "LR", False, "tfMRI_SOCIAL_LR.nii.gz"), + ("SOCIAL", "RL", False, "tfMRI_SOCIAL_RL.nii.gz"), + ("WM", "LR", False, "tfMRI_WM_LR.nii.gz"), + ("WM", "RL", False, "tfMRI_WM_RL.nii.gz"), + ("RELATIONAL", "LR", False, "tfMRI_RELATIONAL_LR.nii.gz"), + ("RELATIONAL", "RL", False, "tfMRI_RELATIONAL_RL.nii.gz"), + ("EMOTION", "LR", False, "tfMRI_EMOTION_LR.nii.gz"), + ("EMOTION", "RL", False, "tfMRI_EMOTION_RL.nii.gz"), + ("LANGUAGE", "LR", False, "tfMRI_LANGUAGE_LR.nii.gz"), + ("LANGUAGE", "RL", False, "tfMRI_LANGUAGE_RL.nii.gz"), + ("GAMBLING", "LR", False, "tfMRI_GAMBLING_LR.nii.gz"), + ("GAMBLING", "RL", False, "tfMRI_GAMBLING_RL.nii.gz"), + ("MOTOR", "LR", False, "tfMRI_MOTOR_LR.nii.gz"), + ("MOTOR", "RL", False, "tfMRI_MOTOR_RL.nii.gz"), + ("REST1", "LR", True, "rfMRI_REST1_LR_hp2000_clean.nii.gz"), + ("REST1", "RL", True, "rfMRI_REST1_RL_hp2000_clean.nii.gz"), + ("REST2", "LR", True, "rfMRI_REST2_LR_hp2000_clean.nii.gz"), + ("REST2", "RL", True, "rfMRI_REST2_RL_hp2000_clean.nii.gz"), ], ) def test_hcp1200_datagrabber( hcpdg: DataladHCP1200, tasks: Optional[str], phase_encodings: Optional[str], + ica_fix: bool, expected_path_name: str, ) -> None: """Test HCP1200 datagrabber. @@ -69,6 +74,8 @@ def test_hcp1200_datagrabber( The parametrized tasks. phase_encodings : str The parametrized phase encodings. + ica_fix : bool + The parametrized ICA-FIX flag. expected_path_name : str The parametrized expected path name. @@ -78,6 +85,7 @@ def test_hcp1200_datagrabber( datadir=hcpdg.datadir, tasks=tasks, phase_encodings=phase_encodings, + ica_fix=ica_fix, ) # Get all elements all_elements = dg.get_elements() @@ -342,3 +350,38 @@ def test_hcp1200_datagrabber_elements( assert element[2] in ["LR", "RL"] assert set(found_subjects) == set(expected_subjects) + + +@pytest.mark.parametrize( + "tasks, ica_fix", + [ + ("SOCIAL", True), + ("WM", True), + ("RELATIONAL", True), + ("EMOTION", True), + ("LANGUAGE", True), + ("GAMBLING", True), + ("MOTOR", True), + ], +) +def test_hcp1200_datagrabber_incorrect_access_icafix( + tasks: Optional[str], + ica_fix: bool +) -> None: + """Test HCP1200 datagrabber incorrect access for icafix. + + Parameters + ---------- + tasks : str + The parametrized tasks. + ica_fix : bool + The parametrized ICA-FIX flag. + + """ + configure_logging(level="DEBUG") + with pytest.raises(ValueError, match="is only available for"): + _ = HCP1200( + datadir=".", + tasks=tasks, + ica_fix=ica_fix, + ) -- 2.52.0 From 80d0a824b372aaee7d32d3b57c7da964aea702ae Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 30 Mar 2023 15:57:53 +0200 Subject: [PATCH 3/5] add news --- docs/changes/newsfragments/183.bugfix | 1 + docs/changes/newsfragments/183.change | 1 + 2 files changed, 2 insertions(+) create mode 100644 docs/changes/newsfragments/183.bugfix create mode 100644 docs/changes/newsfragments/183.change diff --git a/docs/changes/newsfragments/183.bugfix b/docs/changes/newsfragments/183.bugfix new file mode 100644 index 000000000..61760611a --- /dev/null +++ b/docs/changes/newsfragments/183.bugfix @@ -0,0 +1 @@ +Fix a bug in which only `REST1` and `REST2` tasks could be accesed in :class:`.DataladHCP1200` and :class:`.HCP1200` datagrabbers by `Fede Raimondo`_ \ No newline at end of file diff --git a/docs/changes/newsfragments/183.change b/docs/changes/newsfragments/183.change new file mode 100644 index 000000000..374b56acd --- /dev/null +++ b/docs/changes/newsfragments/183.change @@ -0,0 +1 @@ +Add ``ica_fix`` parameter to :class:`.DataladHCP1200` and :class:`.HCP1200` datagrabbers to allow for selecting data processed with ICA+FIX. Default value is ``False`` which changes behaviour since 0.0.1 release. By `Fede Raimondo`_ \ No newline at end of file -- 2.52.0 From 5ca10f1135ce2424d753f9c0a0c7251c13460fc5 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 31 Mar 2023 11:30:09 +0200 Subject: [PATCH 4/5] address comments --- docs/changes/newsfragments/183.bugfix | 2 +- junifer/datagrabber/hcp.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/changes/newsfragments/183.bugfix b/docs/changes/newsfragments/183.bugfix index 61760611a..a2a0c7bb6 100644 --- a/docs/changes/newsfragments/183.bugfix +++ b/docs/changes/newsfragments/183.bugfix @@ -1 +1 @@ -Fix a bug in which only `REST1` and `REST2` tasks could be accesed in :class:`.DataladHCP1200` and :class:`.HCP1200` datagrabbers by `Fede Raimondo`_ \ No newline at end of file +Fix a bug in which only ``REST1`` and ``REST2`` tasks could be accesed in :class:`.DataladHCP1200` and :class:`.HCP1200` datagrabbers by `Fede Raimondo`_ \ No newline at end of file diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index e5a35053c..b9df5e274 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -27,7 +27,7 @@ class HCP1200(PatternDataGrabber): HCP phase encoding directions. If None, both will be used (default None). icafix : bool, optional - Whether to use retrieve data that was processed with ICA+FIX. + Whether to retrieve data that was processed with ICA+FIX. Only 'REST1' and 'REST2' tasks are available with ICA+FIX (default False). **kwargs @@ -86,12 +86,12 @@ class HCP1200(PatternDataGrabber): f"{all_phase_encodings}." ) - if ica_fix is True: + if ica_fix: if not all([task in ["REST1", "REST2"] for task in self.tasks]): raise_error( "ICA+FIX is only available for 'REST1' and 'REST2' tasks." ) - suffix = "_hp2000_clean" if ica_fix is True else "" + suffix = "_hp2000_clean" if ica_fix else "" # The types of data types = ["BOLD"] # The patterns @@ -184,8 +184,8 @@ class DataladHCP1200(DataladDataGrabber, HCP1200): phase_encodings : {"LR", "RL"} or list of the options, optional HCP phase encoding directions. If None, both will be used (default None). - icafix : bool, optional - Whether to use retrieve data that was processed with ICA+FIX. + ica_fix : bool, optional + Whether to retrieve data that was processed with ICA+FIX. Only 'REST1' and 'REST2' tasks are available with ICA+FIX (default False). """ -- 2.52.0 From e56480fedd574f687ad1534d97f1300edd4bd1b2 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Fri, 31 Mar 2023 11:34:08 +0200 Subject: [PATCH 5/5] fix typo --- junifer/datagrabber/hcp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index b9df5e274..d36e74b61 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -26,7 +26,7 @@ class HCP1200(PatternDataGrabber): phase_encodings : {"LR", "RL"} or list of the options, optional HCP phase encoding directions. If None, both will be used (default None). - icafix : bool, optional + ica_fix : bool, optional Whether to retrieve data that was processed with ICA+FIX. Only 'REST1' and 'REST2' tasks are available with ICA+FIX (default False). -- 2.52.0