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

FIX: Repair search for precomputed transforms #3369

Merged
merged 15 commits into from
Oct 2, 2024

Conversation

psadil
Copy link
Contributor

@psadil psadil commented Oct 2, 2024

Changes proposed in this pull request

Closes #3368

This required massaging several kinks from collect_derivatives, so careful review of that would be appreciated.

Documentation that should be reviewed

Copy link

welcome bot commented Oct 2, 2024

Thanks for opening this pull request! It looks like this is your first time contributing to fMRIPrep. 😄
We invite you to list yourself as an fMRIPrep contributor. To learn more about what that entails and how we credit our contributors, please check out the contributing guidelines. If your name is not already on the list, please insert it, in alphabetical order, into the CONTRIBUTORS.md file.
Of course, if you want to opt-out this time there is no problem at all with adding your name later. You will be always welcome to add it in the future whenever you feel it should be listed.

Copy link

codecov bot commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.59%. Comparing base (73189de) to head (1a9ccce).
Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3369      +/-   ##
==========================================
+ Coverage   71.03%   71.59%   +0.55%     
==========================================
  Files          56       57       +1     
  Lines        4233     4249      +16     
  Branches      638      639       +1     
==========================================
+ Hits         3007     3042      +35     
+ Misses       1114     1091      -23     
- Partials      112      116       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Some comments on the test:

fmriprep/utils/tests/test_derivative_cache.py Outdated Show resolved Hide resolved
fmriprep/utils/tests/test_derivative_cache.py Outdated Show resolved Hide resolved
fmriprep/utils/tests/test_derivative_cache.py Outdated Show resolved Hide resolved
fmriprep/utils/bids.py Outdated Show resolved Hide resolved
psadil and others added 5 commits October 2, 2024 09:28
Avoid possible bugs in `io_spec.json` during test

Co-authored-by: Chris Markiewicz <[email protected]>
Simplify assertion in test for loaded transforms

Co-authored-by: Chris Markiewicz <[email protected]>
…eparing to call collect_derivatives

Co-authored-by: Chris Markiewicz <[email protected]>
@effigies
Copy link
Member

effigies commented Oct 2, 2024

For remaining issues, ruff check --fix && ruff format

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

LGTM. Would you mind writing a similar test for the boldref volumes? Can be in this PR or separate. I think we'll need to flip the direction of the q/entities again.

@psadil
Copy link
Contributor Author

psadil commented Oct 2, 2024

LGTM. Would you mind writing a similar test for the boldref volumes? Can be in this PR or separate. I think we'll need to flip the direction of the q/entities again.

👍🏻 #3370

@effigies effigies merged commit ed5794f into nipreps:master Oct 2, 2024
18 checks passed
@psadil psadil deleted the fix/derivatives-xfm-from branch October 2, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants