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] Optionally support echo entity for VFA suffix #989

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

TheChymera
Copy link
Collaborator

@TheChymera TheChymera commented Jan 25, 2022

@effigies this is pertaining to: #977 (comment)

@sappelhoff The PR text told me to ping you for questions.
Should I add a Closes: line to the commit, will this automatically close the issue once merged?

Closes #977.

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@f888291). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #989   +/-   ##
=========================================
  Coverage          ?   36.16%           
=========================================
  Files             ?        8           
  Lines             ?      788           
  Branches          ?        0           
=========================================
  Hits              ?      285           
  Misses            ?      503           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f888291...31d72b3. Read the comment docs.

@effigies
Copy link
Collaborator

Added the closes line to the PR text.

@TheChymera
Copy link
Collaborator Author

@effigies added

@tsalo tsalo added schema Issues related to the YAML schema representation of the specification. Patch version release. structural MRI labels Jan 26, 2022
Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

The schema change looks good. My only question is, are there any changes/updates that should be made to the qMRI appendix page (https://bids-specification.readthedocs.io/en/latest/99-appendices/11-qmri.html)? For example, there's a table that links "file collection" suffixes with qMRI suffixes, and VFA is listed there. Should a new row be added with the appropriate applications for VFA acquisitions with multiple echoes?

@TheChymera
Copy link
Collaborator Author

TheChymera commented Jan 29, 2022

@tsalo I don't think this would be required at this point, and if it would be in the future, I presume the data would no longer be _VFA, but something else.

The tables in this appendix catalog applications where the use of a file collection is REQUIRED

REQUIRED doesn't really fit the bill.
The VFA specification indicates the collection is flip-angle relative, which remains the respective application. Multiple echoes can be added for denoising, or can selectively be varied for select flip angles to provide better signal — or, of course, for explorative analysis.

@sappelhoff sappelhoff merged commit 2507964 into bids-standard:master Feb 2, 2022
@TheChymera TheChymera deleted the echo_for_VFA branch February 4, 2022 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema Issues related to the YAML schema representation of the specification. Patch version release. structural MRI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BIDS VFA anat datatype entry.
4 participants