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 for "echo" entity to be used with T1w and other nonparametric anatomical suffixes #1570

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

yarikoptic
Copy link
Collaborator

Closes #654

#654 provides more of discussion with a number of people bringing up the use cases which keep coming about.

As EchoTime is a recommended sidecar file metadata field for any file of 'mri' modality, I decided that it would only be logical for at least existing use cases (T1w) and possibly other cases (to not overspecify T1w) to simply allow for "echo" entity to be used if so necessary to distignuish two files while they do remain of that weighting (e.g. T1w) given the EchoTime values.

…metric) sequences

Closes bids-standard#654

bids-standard#654 provides more of
discussion with a number of people bringing up the  use cases which keep coming about.

As EchoTime is a recommended sidecar file metadata field for any file of 'mri' modality,
I decided that it would only be logical for at least existing use cases (T1w) and possibly
other cases (to not overspecify T1w) to simply allow for "echo" entity to be used if so
necessary to distignuish two files while they do remain of that weighting (e.g. T1w) given
the EchoTime values.
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d6e77f1) 87.83% compared to head (c5d4501) 87.83%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1570   +/-   ##
=======================================
  Coverage   87.83%   87.83%           
=======================================
  Files          16       16           
  Lines        1356     1356           
=======================================
  Hits         1191     1191           
  Misses        165      165           

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

Copy link
Collaborator

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

Based on @yarikoptic and @neurolabusc's latest comments[0][1] on #654, I think this is a reasonable change.

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 discussion in #654 does seem to have converged around T1w instead of MEGRE, so this change looks good to me.

@effigies effigies changed the title Allow for "echo" entity to be used with T1w (and other "anat" nonparametric) sequences [ENH] Allow for "echo" entity to be used with T1w and other nonparametric anatomical suffixes Aug 24, 2023
@effigies
Copy link
Collaborator

The aye's have it.

@effigies effigies merged commit a98160a into bids-standard:master Aug 24, 2023
19 checks passed
@pwighton
Copy link

I'm a little late chiming in, but I also support this change.

If there is confusion what the weighting (t1, t2, pd) means when dealing with multiple echos, I think we can say the weighting refers to the first (and highest SNR) echo which, to my knowledge, is the common convention.

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.

Add support for multi-echo T1w scans
4 participants