-
Notifications
You must be signed in to change notification settings - Fork 190
[FIX][SCHEMA] Updating <properties:> to enable easy object recursion #2175
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
Conversation
use references for properties objects. Done for EditPulse and Code (something)
finished updating properties for GeneratedBy
Final <properties:> update
for more information, see https://pre-commit.ci
|
I'm not sure about the intricacies of formatting for objects. But would people consider removing the recommended: and required: tags. Instead moving the level next to the key, or as a level: field. This would then have the same format as in rule files. (i.e.: MRIAnatomyCommonMetadataFields: selectors:
- datatype == "anat"
- match(extension, "^\.nii(\.gz)?$")
fields:
ContrastBolusIngredient: optional
RepetitionTimeExcitation: optional
RepetitionTimePreparation: optional). Similarly if all object keys are now on objects.metadata, recommended:
- OperatingSystem
- ScreenDistance
- ScreenRefreshRate
- ScreenResolution
- ScreenSize
- SoftwareName
- SoftwareRRID
- SoftwareVersion
- Code
- HeadStabilization
properties:
OperatingSystem:
$ref: objects.metadata.OperatingSystem
ScreenDistance:
$ref: objects.metadata.ScreenDistance
ScreenRefreshRate:
$ref: objects.metadata.ScreenRefreshRatecould be changed to being: properties:
OperatingSystem: recommended
ScreenDistance: recommended
ScreenRefreshRate: recommendedetc.. Once again encouraging machine readability, where scripts to interpret rule .yamls could be reused for object .yamls. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2175 +/- ##
=======================================
Coverage 82.71% 82.71%
=======================================
Files 20 20
Lines 1608 1608
=======================================
Hits 1330 1330
Misses 278 278 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The reason for |
src/schema/objects/metadata.yaml
Outdated
| required: [PipelineName] | ||
| recommended: [PipelineVersion] | ||
| properties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change. Factoring out subfields should not have any impact on names or semantics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed... Will this not give character case errors as now attributes like description: and name: may clash with metadata definitions Description: and Name:
revert breaking changes.
Ah this makes sense... Is this done to validate actual BIDS dataset JSON files? Or in a meta-manner to validate the schema itself? |
We use the ajv validator to validate sidecar values, although this has to be done one at a time, since the requirement levels can change, e.g.: bids-specification/src/schema/rules/sidecars/asl.yaml Lines 35 to 59 in 6bbd951
TSVs use columns: bids-specification/src/schema/rules/tabular_data/events.yaml Lines 22 to 30 in 6bbd951
These reference column terms like: bids-specification/src/schema/objects/columns.yaml Lines 2 to 8 in 6bbd951
For this, custom validation is required, since TSVs are nothing but strings. I'm in the process of rewriting that check: For NIfTI checks, we rely on a library to parse the NIfTI header, and populate the NIfTI context: bids-specification/src/schema/meta/context.yaml Lines 306 to 422 in 6bbd951
Then we can use it in checks: bids-specification/src/schema/rules/checks/func.yaml Lines 57 to 67 in 6bbd951
|
For metadata of "type: object". Ensured all keys become metadata objects. This would enable a much more elegant and streamlined design for creating relevant namespaces from metadata, by allowing object keys to be recursively defined as metadata objects themselves.
Making this consistent would make it more machine readable, and allow for treatment of property keys as "strict" objects with an easily findable definition (i.e. shema.objects.metadata.get(name)).
This is sometimes done, i.e. for StimulusPresentation:
But othertimes ignored, i.e. for DeidentificationMethodCodeSequence: