-
Notifications
You must be signed in to change notification settings - Fork 169
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] Check sidecar fields mentioned in rule checks exist in the schema #1917
Conversation
Remi-Gau
commented
Aug 30, 2024
- closes none but relates to [FIX] typo #1910 (comment)
@@ -77,3 +86,67 @@ def test_checks(schema_obj): | |||
def test_expected_failures(expr): | |||
with pytest.raises(ParseException): | |||
expression.parse_string(expr) | |||
|
|||
|
|||
def test_valid_sidecar_field(schema_obj): |
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 sort of totally improvised testing this, so there may be a simpler way to do this.
FYI this actually caught a few typos or things that had not been updated in the schema. |
2b16673
to
cde2ba9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1917 +/- ##
=======================================
Coverage 87.23% 87.23%
=======================================
Files 16 16
Lines 1410 1410
=======================================
Hits 1230 1230
Misses 180 180 ☔ View full report in Codecov by Sentry. |
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.
Largely rewrote to fetch any identifier, whether it's a bare (suffix
), a function name (exists
) or a property (nifti_header.dims
). This way we can add more checks easily at the top level.
I'm not going to bog this PR down with other checks, but, for example, we could make sure that nifti_header
fields exist in schema.meta.context.properties.nifti_header.properties
, or schema.*
can be found in the schema itself.
I didn't find any new typos from what you already found by expanding to all rules that include selectors.
Thanks for the review and expansion. |