-
Notifications
You must be signed in to change notification settings - Fork 169
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
Replace terms "modality_label" and "contrast_label" with "suffix" #571
Comments
Agreed. |
After reading a bit more, it seems like modality is never defined, but is used in three ways- (1) the imaging system (e.g., MRI, EEG), (2) the specific data content (e.g., I think the cleanest solution is to treat In my opinion, this means renaming the folders "Modality agnostic files" and "Modality specific files" to something without the word "modality". What about "technology"? Additionally, the term EDIT: Alternatively, we could edit the definition of suffix in common principles to clarify that suffix refers to the modality. |
+1 on that What about "Data type" instead of "Modality" as we already have them under |
Data types refer to divisions at the folder level though (e.g., |
IMHO it is only the "data type" (see prev comment) notion vs "modality" which is the main conflict here. And they kinda could be flipped back and forth and it would still make sense. Note: we do have
while phrasing description of "suffix" in common principles I et al (in #152) tried to stay away from any specific semantic (such as modality) since IIRC some would not fit nicely "into" its semantic IMHO, e.g. Having said that I indeed most often saw we have one off `_mod-` (only for `_defacemask`) to "absorb" `modality_label` but it is about "modality" since suffix in those files is `modality_label`$> git grep '_mod'
04-modality-specific-files/01-magnetic-resonance-imaging-data.md: sub-<label>[_ses-<label>][_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>][_mod-<label>]_defacemask.nii[.gz]
04-modality-specific-files/01-magnetic-resonance-imaging-data.md:For example, `sub-01_mod-T1w_defacemask.nii.gz`.
schema/entities.yaml: E.g., sub-01_mod-T1w_defacemask.nii.gz. I think |
YES, and it another side-effect of original accent on MRI that multiple data types for MRI ( |
I agree that we need better definitions to distinguish these two concepts. I can open a separate issue about this in the near future. Since "data type" is defined under Common Principles, it seems reasonable to define "modality" there as well. I would even be comfortable just updating the "suffix" definition to say something like "Suffixes generally refer to data modalities." I also still think it would be a good idea to have some kind of definition of "technology", although that can be folded into the planned issue.
I agree. I don't see any problem with one-to-one relationships between the different types, although I still don't want to conflate suffixes with technologies by using unclear terminology.
I am comfortable with the idea of the suffix entity having subclasses, but I have a few problems with how that concept is currently implemented.
|
It looks like we've handled this one over the last couple of years, so I'm going to close it. Further discussion about the modality terminology can go in #593. |
Within the MRI section, two terms are used to indicate suffixes, which I think confuses the issue.
In the "Anatomy imaging data" section, the term
modality_label
is used twice. First in the filename template and again in the table.In the "Task (including resting state) imaging data" section, the term
contrast_label
is used twice. First in the table and again in the filename template.These terms seem to just be one-offs, in that they're limited to their respective sections and not used regularly. Instead, within the common principles, the term
suffix
is used. I think we should simply replace these terms withsuffix
. At minimum, we should probably replace the term "modality" with something else, given that "modality" already has a definition in the specification that is much broader than the way it is used here.There is a proposal in #508 that partially addresses this (see https://github.com/bids-standard/bids-specification/pull/508/files#r445625768 for the relevant lines), but it only deals with
modality_label
. Plus, as far as I can tell, even before 1.4.0,modality_label
was just used in the same two spots and was never actually defined.The text was updated successfully, but these errors were encountered: