bids-specification
bids-specification copied to clipboard
[SCHEMA] Add full object definitions for valid values in schema
Closes #904. This PR adds new information to the schema, but should not affect how it is render in the specification.
Changes proposed:
- Convert certain valid values (enums) in the schema to full objects. Specifically, these object definitions contain "name" and "description" fields. In most cases, they're still nested within the parent object definitions, rather than stored as distinct objects and referenced in the objects that use them.
To do:
- [ ] Ensure that the schema rendering and searching functions can handle the fact that many lists of strings are now lists of objects.
- [ ] Fill in the TODOs. I may need to ask for help in describing some (many?) of the values.
- [x] ~~Think about tackling #912 in this PR as well.~~ Dropped in favor of #921.
Codecov Report
Patch coverage: 79.54% and project coverage change: -0.18 :warning:
Comparison is base (
f2cea0f) 88.00% compared to head (41189dc) 87.83%.
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
| Impacted Files | Coverage Δ | |
|---|---|---|
| tools/schemacode/bidsschematools/render/utils.py | 85.18% <0.00%> (ø) |
|
| tools/schemacode/bidsschematools/schema.py | 79.61% <81.81%> (-0.18%) |
:arrow_down: |
| tools/schemacode/bidsschematools/rules.py | 98.96% <83.33%> (-1.04%) |
:arrow_down: |
| tools/schemacode/bidsschematools/render/text.py | 95.53% <87.50%> (-1.10%) |
:arrow_down: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
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.
@tsalo Sorry, this is a nasty one to try to resolve on my own. Conflicts from both #1230 and #1235.
@effigies I think it's all good now.
Per discussion today, I will (1) resolve conflicts, (2) move the value definitions to a new YAML file, and (3) ensure that there are links in the "allowed values" sections in glossary entries.
@sappelhoff @effigies I've added all the channel types I could find. It looks like there are already enums for valid channels types within the different modalities, so we might actually be more strict than we want to be with enforcement.
I haven't split up the values file at all. Not sure how you'd like to do that.
Thanks for reviewing @sappelhoff!
While other items don't have such a comment, yet they are not converted to the "new form" either
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.
Shouldn't we just add information for all enums (except those that only have "n/a")?
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 handedness column, since they include l, L, left, Left, LEFT, etc.), but I do think most valid values could use a description. I think we can tackle that in future PRs though, since this one adds in the infrastructure we need to make that work.
Thanks!
thanks a lot again @tsalo!