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

feat: Add flag to fallback to Estimated* metadata or a passed value for TotalReadoutTime #3423

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

effigies
Copy link
Member

This flag provides two additional ways to retrieve TotalReadoutTime. If EstimatedTotalReadoutTime or EstimatedEffectiveEchoSpacing are defined, the user can pass --fallback-total-readout-time estimated to direct fMRIPrep to use these.

This could be turned on by default, but I don't think this is prudent. The reason that dcm2niix does not simply say TotalReadoutTime or EffectiveEchoSpacing is not to misrepresent the confidence that these are correct. Previously, users had to actively remove the Estimated to accept the estimates, and I don't think we want to eliminate the conscious decision. However, we can make it so that users do not need to modify their datasets to accept them.

Closes #3009.

@effigies effigies force-pushed the enh/force-total-readout-time branch from a535ac2 to f63204f Compare January 29, 2025 17:37
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.

Project coverage is 72.25%. Comparing base (64e27fb) to head (986528d).

Files with missing lines Patch % Lines
fmriprep/cli/parser.py 33.33% 6 Missing ⚠️
fmriprep/workflows/base.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3423      +/-   ##
==========================================
- Coverage   72.33%   72.25%   -0.08%     
==========================================
  Files          57       57              
  Lines        4269     4282      +13     
  Branches      545      546       +1     
==========================================
+ Hits         3088     3094       +6     
- Misses       1065     1072       +7     
  Partials      116      116              

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

@effigies effigies force-pushed the enh/force-total-readout-time branch from d1fd368 to 986528d Compare January 29, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure per-BOLD SyN-SDC does not require TotalReadoutTime
1 participant