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.

Sign up for GitHub

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

Jump to bottom

FIX: Repair search for precomputed transforms #3369

Merged
merged 15 commits into from
Oct 2, 2024
Merged

FIX: Repair search for precomputed transforms #3369

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

@welcome Welcome
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.

@codecov Codecov
Copy link

codecov bot commented Oct 2, 2024 •
edited
Loading

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.

effigies
effigies reviewed Oct 2, 2024
View reviewed changes
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
effigies
effigies reviewed Oct 2, 2024
View reviewed changes
fmriprep/utils/bids.py Outdated Show resolved Hide resolved
psadil and others added 5 commits October 2, 2024 09:28
@psadil @effigies
Update fmriprep/utils/tests/test_derivative_cache.py
758caaf 
Avoid possible bugs in `io_spec.json` during test

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@psadil @effigies
Update fmriprep/utils/tests/test_derivative_cache.py
bef2a21 
Simplify assertion in test for loaded transforms

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@psadil @effigies
use variables while populating the directory and a dictionary when pr…
97f637a 
…eparing to call collect_derivatives

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@psadil @effigies
Reorder merging of queries to simplify overrides
dc83e12 
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@psadil
remove unused imports; fix variable name
1c95189
@effigies
Copy link
Member

effigies commented Oct 2, 2024

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

effigies
effigies approved these changes Oct 2, 2024
View reviewed changes
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 psadil mentioned this pull request Oct 2, 2024
Merged
@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
effigies pushed a commit that referenced this pull request Oct 2, 2024
@psadil @effigies
FIX: Repair search for precomputed transforms (#3369)
ce31460
effigies added a commit that referenced this pull request Oct 12, 2024
@effigies
Merge tag '24.1.1'
1d6ac2b 
24.1.1 (October 10, 2024)

Bug fix release in the 24.1.x series.

Precomputed functional derivatives were not being correcly detected,
and a couple fixes for rare issues.

  * FIX: Remove checks for unit zooms and symmetric rotations in template warp (#3376)
  * FIX: Stop excluding FS minc_modify_header used during fallback registration (#3372)
  * FIX: Repair search for precomputed bold references (#3370)
  * FIX: Repair search for precomputed transforms (#3369)
tsalo added a commit to tsalo/aslprep that referenced this pull request Nov 4, 2024
@tsalo
Reproduce nipreps/fmriprep#3369.
9b4d5d8
tsalo added a commit to PennLINC/aslprep that referenced this pull request Nov 4, 2024
@tsalo
Bring up to date with fMRIPrep 24.1.1 (#455)
4db9196 
* Update fMRIPrep version.

* Update linting rules.

* Run ruff --fix.

* Fix some style issues.

* Fix remaining ruff issues.

* Update more stuff.

* Update test_cli.py

* Run ruff format.

* Fix stuff.

* Keep bringing it up to date.

* Fix things up some more.

* Keep working.

* Update config.py

* Fix.

* Update base.py

* Update bids.py

* Update.

* Update run.py

* Use init_bold_fsLR_resampling_wf from fMRIPrep.

* Fix generate_reports calls.

* Don't try to invert.

* Use internal copy of generate_reports.

* Add `--aggregate-session-reports`.

* Update report specs.

* Update asl.py

* Fix a couple of things.

* Update io_spec.json

* Reproduce nipreps/fmriprep#3369.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effigies effigies effigies approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@psadil @effigies