-
Notifications
You must be signed in to change notification settings - Fork 163
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
[SCHEMA] Implement some fairly easy rules #1410
[SCHEMA] Implement some fairly easy rules #1410
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #1410 +/- ##
==========================================
+ Coverage 88.00% 88.65% +0.64%
==========================================
Files 14 11 -3
Lines 1267 1084 -183
==========================================
- Hits 1115 961 -154
+ Misses 152 123 -29
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
As much fun to read as it was to write I'm sure.
selectors: | ||
- type(sidecar.VolumeTiming) != null | ||
checks: | ||
- type(sidecar.DelayTime) == null |
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.
I personally prefer the in
notation for clarity. If we wanted to avoid infix notation then I'd prefer the exists
function being extended to support this.
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.
The case of null
being put in an actual json file should be covered by format type checking on sidecar entries.
The image/file will be considered invalid or assumed to be in LAS orientation. | ||
level: warning | ||
selectors: | ||
- nifti_header != null |
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.
nifti_header != null
vs
type(nifti_header) != "null"
should be functionally equivalent, should one style be preferred over the other?
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.
I guess type() should be preferred since it makes it easier for me to control null semantics in the validators expression language implementation.
Builds on #1406. Working through #1402.