[ENH]: Allow users to select `types` for datagrabbers in order to avoid downloading unnecesary data. #132

Merged

synchon merged 18 commits from update/types-for-aomic-dg into main

2023-07-21 11:28:24 +00:00

synchon commented

2023-01-05 10:51:18 +00:00

(Migrated from github.com)

Are you requiring a new dataset or marker?

I understand this is not a marker or dataset request

Which feature do you want to include?

Datalad-based datasets such as DataladAOMICID1000, DataladAOMICPIOP1 and DataladAOMICPIOP2 have several data types including BOLD, T1w, DWI, etc.

So far, the types parameter is not exposed to the constructor, os a user that only requires one data type, will be forced to datalad get all the data.

How do you imagine this integrated in junifer?

easy, as with the tasks parameter, expose the types one.

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

### Are you requiring a new dataset or marker? - [X] I understand this is not a marker or dataset request ### Which feature do you want to include? Datalad-based datasets such as DataladAOMICID1000, DataladAOMICPIOP1 and DataladAOMICPIOP2 have several data types including BOLD, T1w, DWI, etc. So far, the types parameter is not exposed to the constructor, os a user that only requires one data type, will be forced to `datalad get` all the data. ### How do you imagine this integrated in junifer? easy, as with the `tasks` parameter, expose the `types` one. ### Do you have a sample code that implements this outside of junifer? _No response_ ### Anything else to say? _No response_

github-actions[bot] commented

2023-01-05 10:55:56 +00:00

(Migrated from github.com)

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-07-21 11:33 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.4 :---: Preview removed because the pull request was closed. 2023-07-21 11:33 UTC

codecov[bot] commented

2023-01-05 11:11:39 +00:00

(Migrated from github.com)

Codecov Report

Merging #132 (dd06c61) into main (a451a98) will decrease coverage by 0.02%.
The diff coverage is 94.87%.

❗ Current head dd06c61 differs from pull request most recent head 7fa55c6. Consider uploading reports for the commit 7fa55c6 to get more accurate results

@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
- Coverage   93.12%   93.10%   -0.02%     
==========================================
  Files          84       84              
  Lines        3695     3714      +19     
  Branches      709      722      +13     
==========================================
+ Hits         3441     3458      +17     
  Misses        160      160              
- Partials       94       96       +2

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`93.10% <94.87%> (-0.02%)`	⬇️

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

Impacted Files	Coverage Δ
junifer/datagrabber/aomic/piop1.py	`97.61% <90.90%> (-2.39%)`	⬇️
junifer/datagrabber/aomic/piop2.py	`97.43% <94.44%> (-2.57%)`	⬇️
junifer/datagrabber/aomic/id1000.py	`100.00% <100.00%> (ø)`
junifer/datagrabber/utils.py	`100.00% <100.00%> (ø)`

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#132](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (dd06c61) into [main](https://app.codecov.io/gh/juaml/junifer/commit/a451a9823202763919f1321810a6bb6ddbc5feac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (a451a98) will **decrease** coverage by `0.02%`. > The diff coverage is `94.87%`. > :exclamation: Current head dd06c61 differs from pull request most recent head 7fa55c6. Consider uploading reports for the commit 7fa55c6 to get more accurate results [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/132/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://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #132 +/- ## ========================================== - Coverage 93.12% 93.10% -0.02% ========================================== Files 84 84 Lines 3695 3714 +19 Branches 709 722 +13 ========================================== + Hits 3441 3458 +17 Misses 160 160 - Partials 94 96 +2 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `93.10% <94.87%> (-0.02%)` | :arrow_down: | 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://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/datagrabber/aomic/piop1.py](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9waW9wMS5weQ==) | `97.61% <90.90%> (-2.39%)` | :arrow_down: | | [junifer/datagrabber/aomic/piop2.py](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9waW9wMi5weQ==) | `97.43% <94.44%> (-2.57%)` | :arrow_down: | | [junifer/datagrabber/aomic/id1000.py](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9pZDEwMDAucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/datagrabber/utils.py](https://app.codecov.io/gh/juaml/junifer/pull/132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci91dGlscy5weQ==) | `100.00% <100.00%> (ø)` | |

fraimondo (Migrated from github.com) requested changes

2023-07-14 19:24:22 +00:00

fraimondo (Migrated from github.com) left a comment

Can we add a test to check what happens when the user passes a type that is not defined in the datagrabber? I guess the validate_patterns should fail.

Can we add a test to check what happens when the user passes a type that is not defined in the datagrabber? I guess the `validate_patterns` should fail.

synchon commented

2023-07-17 08:13:57 +00:00

(Migrated from github.com)

Can we add a test to check what happens when the user passes a type that is not defined in the datagrabber? I guess the validate_patterns should fail.

I believe they are there, can you please check the tests with the name like test_<datagrabber>_incorrect_data_type()?

> Can we add a test to check what happens when the user passes a type that is not defined in the datagrabber? I guess the `validate_patterns` should fail. I believe they are there, can you please check the tests with the name like `test_<datagrabber>_incorrect_data_type()`?

fraimondo (Migrated from github.com) approved these changes

2023-07-21 11:27:22 +00:00