[BUG]: HCP1200 datagrabber does not provide the correct BOLD files #183

2023-03-30T13:53:12Z

fraimondo commented

2023-03-30 13:53:12 +00:00

(Migrated from github.com)

Is there an existing issue for this?

I have searched the existing issues

Current Behavior

HCP1200 preprocessed BOLD data has the ICA+FIX preprocessed data. By default, we are providing this as BOLD. However, this is only present in the REST task, so it fails to find the files from other tasks.

This also means that the testing dataset in GIN is wrong.

Expected Behavior

The user should get the right files, even for NON REST tasks.

Steps To Reproduce

Use the HCP1200 datagrabber with a task to get bold data.

Environment

junifer:
  version: 0.0.2.dev78
python:
  version: 3.11.0
  implementation: CPython
dependencies:
  click: 8.1.3
  numpy: 1.23.4
  datalad: 0.17.9
  pandas: 1.5.1
  nibabel: 4.0.2
  nilearn: 0.9.2
  sqlalchemy: 1.4.43
  yaml: '6.0'
system:
  platform: macOS-13.1-x86_64-i386-64bit
environment:
  LC_CTYPE: UTF-8
  PATH: /Users/fraimondo/Applications/workbench/bin_macosx64:/Users/fraimondo/.rbenv/shims:/Users/fraimondo/dev/tbox/freesurfer/bin:/Users/fraimondo/dev/tbox/freesurfer/fsfast/bin:/Users/fraimondo/dev/tbox/fsl/bin:/Users/fraimondo/dev/tbox/freesurfer/mni/bin:/usr/local/opt/ruby/bin:/Users/fraimondo/miniconda3/envs/junifer/bin:/Users/fraimondo/miniconda3/condabin:/Users/fraimondo/dev/tbox/fsl/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/TeX/texbin:/opt/X11/bin:/Library/Apple/usr/bin:/Users/fraimondo/.platformio/penv/bin:/Users/fraimondo/gems/bin:/Users/fraimondo/.gem/ruby/3.0.0/bin:/Users/fraimondo/dev/tbox/junifer/junifer/api/res/afni

Relevant log output

No response

Anything else?

Proposed solution:

add a parameter to the HCP1200 datagrabber constructor (ica_fix) with a default of False.
in the suffix, if ica_fix is True, then add the hp1200_clean suffix to the filename, something like this

            suffix = "_hp2000_clean" if ica_fix is True else ""
            "BOLD": (
                "{subject}/MNINonLinear/Results/"
                "{task}_{phase_encoding}/"
                "{task}_{phase_encoding}{suffix}.nii.gz"
            )

Fix the HCP1200 testing dataset in gin to add the non hp1200_clean files and remove them from the non REST tasks.

### Is there an existing issue for this? - [X] I have searched the existing issues ### Current Behavior HCP1200 preprocessed BOLD data has the ICA+FIX preprocessed data. By default, we are providing this as BOLD. However, this is only present in the REST task, so it fails to find the files from other tasks. This also means that the testing dataset in GIN is wrong. ### Expected Behavior The user should get the right files, even for NON REST tasks. ### Steps To Reproduce 1. Use the HCP1200 datagrabber with a task to get bold data. ### Environment ```markdown junifer: version: 0.0.2.dev78 python: version: 3.11.0 implementation: CPython dependencies: click: 8.1.3 numpy: 1.23.4 datalad: 0.17.9 pandas: 1.5.1 nibabel: 4.0.2 nilearn: 0.9.2 sqlalchemy: 1.4.43 yaml: '6.0' system: platform: macOS-13.1-x86_64-i386-64bit environment: LC_CTYPE: UTF-8 PATH: /Users/fraimondo/Applications/workbench/bin_macosx64:/Users/fraimondo/.rbenv/shims:/Users/fraimondo/dev/tbox/freesurfer/bin:/Users/fraimondo/dev/tbox/freesurfer/fsfast/bin:/Users/fraimondo/dev/tbox/fsl/bin:/Users/fraimondo/dev/tbox/freesurfer/mni/bin:/usr/local/opt/ruby/bin:/Users/fraimondo/miniconda3/envs/junifer/bin:/Users/fraimondo/miniconda3/condabin:/Users/fraimondo/dev/tbox/fsl/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/TeX/texbin:/opt/X11/bin:/Library/Apple/usr/bin:/Users/fraimondo/.platformio/penv/bin:/Users/fraimondo/gems/bin:/Users/fraimondo/.gem/ruby/3.0.0/bin:/Users/fraimondo/dev/tbox/junifer/junifer/api/res/afni ``` ### Relevant log output _No response_ ### Anything else? Proposed solution: 1) add a parameter to the HCP1200 datagrabber constructor (`ica_fix`) with a default of `False`. 2) in the suffix, if `ica_fix` is True, then add the `hp1200_clean` suffix to the filename, something like this ```Python suffix = "_hp2000_clean" if ica_fix is True else "" "BOLD": ( "{subject}/MNINonLinear/Results/" "{task}_{phase_encoding}/" "{task}_{phase_encoding}{suffix}.nii.gz" ) ``` 3) Fix the HCP1200 testing dataset in gin to add the non hp1200_clean files and remove them from the non REST tasks.

codecov[bot] commented

2023-03-30 13:55:56 +00:00

(Migrated from github.com)

Codecov Report

Merging #183 (e56480f) into main (ace98da) will increase coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
+ Coverage   93.30%   93.61%   +0.31%     
==========================================
  Files          79       80       +1     
  Lines        3403     3461      +58     
  Branches      642      653      +11     
==========================================
+ Hits         3175     3240      +65     
+ Misses        150      144       -6     
+ Partials       78       77       -1

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`93.60% <100.00%> (+0.31%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
junifer/datagrabber/hcp.py	`100.00% <100.00%> (ø)`

... and 18 files with indirect coverage changes

## [Codecov](https://codecov.io/gh/juaml/junifer/pull/183?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#183](https://codecov.io/gh/juaml/junifer/pull/183?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (e56480f) into [main](https://codecov.io/gh/juaml/junifer/commit/ace98da340e20cffcd2a5d939ff0d57afe4451d0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (ace98da) will **increase** coverage by `0.31%`. > The diff coverage is `100.00%`. [![Impacted file tree graph](https://codecov.io/gh/juaml/junifer/pull/183/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://codecov.io/gh/juaml/junifer/pull/183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #183 +/- ## ========================================== + Coverage 93.30% 93.61% +0.31% ========================================== Files 79 80 +1 Lines 3403 3461 +58 Branches 642 653 +11 ========================================== + Hits 3175 3240 +65 + Misses 150 144 -6 + Partials 78 77 -1 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `93.60% <100.00%> (+0.31%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/juaml/junifer/pull/183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/datagrabber/hcp.py](https://codecov.io/gh/juaml/junifer/pull/183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9oY3AucHk=) | `100.00% <100.00%> (ø)` | | ... and [18 files with indirect coverage changes](https://codecov.io/gh/juaml/junifer/pull/183/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)

github-actions[bot] commented

2023-03-30 14:03:09 +00:00

(Migrated from github.com)

PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-31 10:17 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.3.0 :---: Preview removed because the pull request was closed. 2023-03-31 10:17 UTC

synchon (Migrated from github.com) requested changes

2023-03-31 08:52:13 +00:00

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`_

``REST1`` and ``REST2``

``` ``REST1`` and ``REST2`` ```

junifer/datagrabber/hcp.py

					
				@ -26,9 +26,12 @@ class HCP1200(PatternDataGrabber):

				    phase_encodings : {"LR", "RL"} or list of the options, optional

Whether to retrieve data ...

junifer/datagrabber/hcp.py

					
				@ -82,6 +86,12 @@ class HCP1200(PatternDataGrabber):

				                    f"{all_phase_encodings}."

if ica_fix:?

```if ica_fix:```?

... if ica_fix else ...?

```... if ica_fix else ...```?

junifer/datagrabber/hcp.py

					
				@ -173,6 +184,10 @@ class DataladHCP1200(DataladDataGrabber, HCP1200):

				    phase_encodings : {"LR", "RL"} or list of the options, optional

icafix => ica_fix

Whether to retrieve data ...

fraimondo (Migrated from github.com) reviewed

2023-03-31 09:29:15 +00:00

junifer/datagrabber/hcp.py

					
				@ -26,9 +26,12 @@ class HCP1200(PatternDataGrabber):

				    phase_encodings : {"LR", "RL"} or list of the options, optional

damn copilot

synchon (Migrated from github.com) approved these changes

2023-03-31 09:35:09 +00:00

Sign in to join this conversation.

No reviewers

synchon

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

juaml/junifer!183

				`@ -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`_

				`@ -26,9 +26,12 @@ class HCP1200(PatternDataGrabber):`
				`phase_encodings : {"LR", "RL"} or list of the options, optional`

				`@ -82,6 +86,12 @@ class HCP1200(PatternDataGrabber):`
				`f"{all_phase_encodings}."`

				`@ -173,6 +184,10 @@ class DataladHCP1200(DataladDataGrabber, HCP1200):`
				`phase_encodings : {"LR", "RL"} or list of the options, optional`

Rows
Columns