[SCHEMA] Add full object definitions for valid values in schema#919
[SCHEMA] Add full object definitions for valid values in schema#919sappelhoff merged 66 commits intobids-standard:masterfrom
Conversation
| Type the gradient pulses used in the `"control"` condition: | ||
| `"balanced"` or `"unbalanced"`. | ||
| type: string | ||
| # TODO: Add definitions for these values. |
There was a problem hiding this comment.
What do the different PCASLType values mean?
| Values MUST be one of `"blood"`, `"saliva"`, `"brain"`, `"csf"`, | ||
| `"breast milk"`, `"bile"`, `"amniotic fluid"`, `"other biospecimen"`. | ||
| type: string | ||
| # TODO: Add definitions for these values. |
There was a problem hiding this comment.
Should we describe the different SampleOrigin values?
| Specifies which spoiling method(s) are used by a spoiled sequence. | ||
| Accepted values: `"RF"`, `"GRADIENT"` or `"COMBINED"`. | ||
| type: string | ||
| # TODO: Add definitions for these values. |
There was a problem hiding this comment.
What do the different SpoilingType values mean?
| Values MUST be one of `"gray matter"`, `"white matter"`, `"csf"`, | ||
| `"meninges"`, `"macrovascular"` or `microvascular`. | ||
| type: string | ||
| # TODO: Add definitions for these values. |
There was a problem hiding this comment.
Should we describe the different TissueOrigin values?
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #919 +/- ##
==========================================
- Coverage 88.00% 87.83% -0.18%
==========================================
Files 16 16
Lines 1326 1356 +30
==========================================
+ Hits 1167 1191 +24
- Misses 159 165 +6
☔ View full report in Codecov by Sentry. |
I'm not 100% sure about this one. I grabbed values from the coordinate systems appendix.
src/schema/objects/metadata.yaml
Outdated
| _NonstandardTemplateCoordSys: | ||
| type: string | ||
| enum: | ||
| - individual: | ||
| name: individual | ||
| description: | | ||
| Participant specific anatomical space (for example derived from T1w and/or T2w images). | ||
| This coordinate system requires specifying an additional, participant-specific file to be fully defined. | ||
| In context of surfaces this space has been referred to as `fsnative`. | ||
|
|
||
| In order for this space to be interpretable, `SpatialReference` metadata MUST be provided. | ||
| - study: | ||
| name: study | ||
| description: | | ||
| Custom space defined using a group/study-specific template. | ||
| This coordinate system requires specifying an additional file to be fully defined. | ||
|
|
||
| In order for this space to be interpretable, `SpatialReference` metadata MUST be provided. | ||
| _NonTemplateCoordSys: | ||
| type: string | ||
| enum: | ||
| - scanner: | ||
| name: scanner | ||
| description: | | ||
| The intrinsic coordinate system of the original image (the first entry of `RawSources`) | ||
| after reconstruction and conversion to NIfTI or equivalent for the case of surfaces and dual volume/surface files. | ||
|
|
||
| The `scanner` coordinate system is implicit and assumed by default if the derivative filename does not | ||
| define **any** `space-<label>`. | ||
| Please note that `space-scanner` SHOULD NOT be used, | ||
| it is mentioned in this specification to make its existence explicit. |
There was a problem hiding this comment.
I'm not sure if these values should be enumerated here. I took them from https://bids-specification.readthedocs.io/en/stable/99-appendices/08-coordinate-systems.html#nonstandard-coordinate-system-identifiers and https://bids-specification.readthedocs.io/en/stable/99-appendices/08-coordinate-systems.html#non-template-coordinate-system-identifiers.
|
Noting that this would be a backwards incompatible change (cf #1013), but easily handled by detecting a list of single-keyed objects to a list of keys. |
for more information, see https://pre-commit.ci
…ification into enum-definitions
effigies
left a comment
There was a problem hiding this comment.
The Phase/SliceEncodingDirection values are slightly weirdly worded. Would something like this work?
Alternately, we at least should use phase/slice in both lines.
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
There was a problem hiding this comment.
I have reviewed this PR and I approve of it, thanks a lot for this work.
Just one question:
For some items you have added the comment
# TODO: Add definitions for these values.
While other items don't have such a comment, yet they are not converted to the "new form" either, e.g.:
enum:
- IODINE
- GADOLINIUM
- CARBON DIOXIDE
- BARIUM
- XENON
Shouldn't we just add information for all enums (except those that only have "n/a")?
Or is that kind of superfluous? (e.g., for m, cm, mm enums ... meter, centimeter ...)
|
Thanks for reviewing @sappelhoff!
Those could be enums that were added after I started this PR, or they might even be ones I just missed in my original pass-through.
I think some are self-explanatory (e.g., I don't know if I want to write descriptions for all of the allowed values for the |
sappelhoff
left a comment
There was a problem hiding this comment.
ok, great. I am going to merge this today and then do follow up PRs:
- rendering the channel type tables in the modality sections from the schema
- creating a new appendix page rendering ALL channel types (and putting links to this appendix below the modality ch type tables)
- adding remaining enums (or adding comments saying that we won't add them as of now ... for cases where such comments are not currently there already)
|
Thanks! |
|
thanks a lot again @tsalo! |
This PR adds new information to the schema, but should not affect how it is rendered in the specification.
will simplify finding a solution for:
channels.tsvfiles. #1436I'm not going to write a rendering function based on tags in this PR. I think it can go in a future one.
Changes proposed:
To do:
Think about tackling Render valid value restrictions in tables based on object definitions in schema #912 in this PR as well.Dropped in favor of [ENH] Render valid value restrictions in tables based on object definitions in schema #921.