[SCHEMA] Define YAML tables for MRI common metadata fields and anatomy data #1017
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tsalo
commented
Feb 24, 2022
| @@ -61,21 +63,21 @@ MRISequenceSpecifics: | |||
|
|
|||
| PETMRISequenceSpecifics: | |||
| selectors: | |||
| - modality == "MRI" | |||
| - "PET" in dataset.modalities | |||
There was a problem hiding this comment.
Having quotes around PET causes syntax error: expected <block end>, but found '<scalar>' (syntax)
tsalo
commented
Feb 24, 2022
| RepetitionTimeExcitation: OPTIONAL | ||
| RepetitionTimePreparation: OPTIONAL | ||
|
|
||
| # Entities |
There was a problem hiding this comment.
I added these because they weren't covered in #1014, but I'm not sure if they should go in this file.
There was a problem hiding this comment.
This feels like it should be its own file, but it does need to be highest priority.
Only issue I'm seeing is that these will correspond to data files, and not, say, an events.tsv.
tsalo
commented
Mar 3, 2022
src/schema/rules/sidecars.yml
Outdated
| selectors: | ||
| - modality == "MRI" | ||
| # NOTE: This is my idea for "field is defined", but I don't know if it's the best approach. | ||
| - sidecar.DelayTime != "n/a" |
There was a problem hiding this comment.
From today's call, we can create a keyword for "is defined".
tsalo
commented
Mar 3, 2022
effigies
reviewed
Mar 10, 2022
src/schema/rules/sidecars.yml
Outdated
| - modality == "MRI" | ||
| - "PET" in dataset.modalities | ||
| - modality == "MRI" | ||
| - PET in dataset.modalities |
There was a problem hiding this comment.
What about the inverse?
Suggested change
| - PET in dataset.modalities | |
| - dataset.modalities contains "PET" |
effigies
reviewed
Mar 10, 2022
src/schema/rules/sidecars.yml
Outdated
| SliceTimingSparse: | ||
| selectors: | ||
| - modality == "MRI" | ||
| - sidecar.DelayTime isDefined |
There was a problem hiding this comment.
Just a thought:
Suggested change
| - sidecar.DelayTime isDefined | |
| - sidecar contains "DelayTime" |
Or maybe
Suggested change
| - sidecar.DelayTime isDefined | |
| - defined(sidecar.DelayTime) |
22 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References #1014 and #1002.
Changes proposed: