Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
Jul 21, 2023

Conversation

synchon
Copy link
Member

@synchon synchon commented Jan 5, 2023

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

@fraimondo fraimondo added enhancement New feature or request triage New issues waiting to be reviewed labels Nov 16, 2022
@synchon synchon added WIP Work in progress and removed triage New issues waiting to be reviewed labels Jan 3, 2023
@synchon synchon added this to the 0.0.2 (alpha 1) milestone Jan 3, 2023
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

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

@codecov
Copy link

codecov bot commented Jan 5, 2023

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

Impacted file tree graph

@@            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%> (ø)

@fraimondo fraimondo marked this pull request as draft March 30, 2023 16:28
@synchon synchon force-pushed the update/types-for-aomic-dg branch from 80c4e56 to 89a6819 Compare May 3, 2023 08:44
@synchon synchon added the dataset Issues or pull requests related to datagrabbers or datasets label Jul 7, 2023
@synchon synchon marked this pull request as ready for review July 14, 2023 12:47
@synchon synchon added ready Pull request is ready for review and merge and removed WIP Work in progress labels Jul 14, 2023
@synchon synchon requested a review from fraimondo July 14, 2023 12:48
Copy link
Contributor

@fraimondo fraimondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

synchon commented Jul 17, 2023

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()?

@synchon synchon merged commit 5a46fb5 into main Jul 21, 2023
24 checks passed
@synchon synchon deleted the update/types-for-aomic-dg branch July 21, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dataset Issues or pull requests related to datagrabbers or datasets enhancement New feature or request ready Pull request is ready for review and merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants