Skip to content
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

Use conditional schemas for domain types to improve error messsages #9

Merged
merged 1 commit into from
Dec 3, 2021

Conversation

letmaik
Copy link
Member

@letmaik letmaik commented Dec 2, 2021

This makes a schema change for domain types using a combination of dependentSchemas and if/then. The errors are massively better and I think there's only minor overhead in writing the schema. Once json-schema-org/json-schema-spec#1082 is a thing we can move to that which adds more syntax sugar for this case.

Before:

E           jsonschema.exceptions.ValidationError: {'type': 'Domain', 'domainType': 'Grid', 'axes': {'y': {'values': [1, 2]}, 't': {'values': ['2008-01-01T04:00:00Z', '2008-01-01T05:00:00Z']}}, 'referencing': [{'coordinates': ['t'], 'system': {'type': 'TemporalRS', 'calendar': 'Gregorian'}}, {'coordinates': ['x', 'y'], 'system': {'type': 'GeographicCRS', 'id': 'http://www.opengis.net/def/crs/OGC/1.3/CRS84'}}]} is not valid under any of the given schemas
E
E           Failed validating 'oneOf' in schema:
E               {'$id': '/schemas/domain',
E                'description': 'A Domain, which defines a set of positions and their '  
E                               'extent in one or more referencing systems',
E                'oneOf': [{'description': 'Grid domain: x and y are required, z and t ' 
E                                          'optional',
E                           'properties': {'axes': {'additionalProperties': False,       
E                                                   'properties': {'t': {'$ref': '/schemas/stringAxis'},
E                                                                  'x': {'$ref': '/schemas/numericAxis'},
E                                                                  'y': {'$ref': '/schemas/numericAxis'},
E                                                                  'z': {'$ref': '/schemas/numericAxis'}},
E                                                   'required': ['x', 'y']},
E                                          'domainType': {'const': 'Grid'}},
E                           'required': ['domainType']},
E                          {'description': 'Trajectory domain: mandatory composite '     
E                                          'axis and optional z axis',
E                           'properties': {'axes': {'additionalProperties': False,       
E                                                   'properties': {'composite': {'$ref': 
'/schemas/tupleAxis'},
E                                                                  'z': {'$ref': '/schemas/numericAxis'}},
E                                                   'required': ['composite']},
E                                          'domainType': {'const': 'Trajectory'}},       
E                           'required': ['domainType']},
E                          {'description': 'Default case: domainType omitted, no '       
E                                          'restrictions on axes',
E                           'not': {'required': ['domainType']},
E                           'properties': {'axes': {'patternProperties': {'.+': {'$ref': 
'/schemas/anyAxis'}}}}}],
E                'properties': {'axes': {'description': 'Set of Axis objects, keyed by ' 
E                                                       'axis identifier',
E                                        'type': 'object'},
E                               'domainType': {'type': 'string'},
E                               'referencing': {'items': {'$ref': '/schemas/referenceSystemConnection'},
E                                               'type': 'array'},
E                               'type': {'const': 'Domain'}},
E                'required': ['type', 'axes'],
E                'type': 'object'}
E
E           On instance:
E               {'axes': {'t': {'values': ['2008-01-01T04:00:00Z',
E                                          '2008-01-01T05:00:00Z']},
E                         'y': {'values': [1, 2]}},
E                'domainType': 'Grid',
E                'referencing': [{'coordinates': ['t'],
E                                 'system': {'calendar': 'Gregorian',
E                                            'type': 'TemporalRS'}},
E                                {'coordinates': ['x', 'y'],
E                                 'system': {'id': 'http://www.opengis.net/def/crs/OGC/1.3/CRS84',
E                                            'type': 'GeographicCRS'}}],
E                'type': 'Domain'}

After:

E           jsonschema.exceptions.ValidationError: 'x' is a required property
E
E           Failed validating 'required' in schema['dependentSchemas']['domainType']['allOf'][0]['then']['properties']['axes']:
E               {'additionalProperties': False,
E                'properties': {'t': {'$ref': '/schemas/stringAxis'},
E                               'x': {'$ref': '/schemas/numericAxis'},
E                               'y': {'$ref': '/schemas/numericAxis'},
E                               'z': {'$ref': '/schemas/numericAxis'}},
E                'required': ['x', 'y']}
E
E           On instance['axes']:
E               {'t': {'values': ['2008-01-01T04:00:00Z']},
E                'y': {'values': [1]},
E                'z': {'values': [1, 2, 3]}}

@jonblower
Copy link
Member

This looks great! I guess we could consider factoring out the domainType sub-schemas into different files and $ref-ing them, but maybe it won't be necessary.

@jonblower jonblower merged commit c1078f2 into main Dec 3, 2021
@jonblower jonblower deleted the letmaik/domain-types-as-conditional-schemas branch December 3, 2021 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants