[config-system] Validator rewrite: Use jsonschema #576
Conversation
Express (in JSON Schema) the constraints of the property black_body_sampling as found in tardis_config_definition.yml.
…sible to specify the abundances.
The YAMLLoader implementation isn't part of the config_reader module anymore.
| montecarlo_section['virtual_spectrum_range']['samples'] = \ | ||
| virtual_spectrum_section[2] |
There was a problem hiding this comment.
What's the reason we keep this?
I propose to delete lines 1031-1046 and rename montecarlo.virtual_spectrum_range.samples to montecarlo.virtual_spectrum_range.num wherever it is accessed
There was a problem hiding this comment.
This is part of the next and final big milestone/PR (configuration system restructure). My purpose in this PR was to only do changes related and absolutely necessary to the validator.
There was a problem hiding this comment.
@yeganer that's dangerous as older configuration files need to work. I would flag it but leave it for now.
There was a problem hiding this comment.
I think we should have a function/module that provides the backwards compatibility.
if config.version < latest_config_version:
config_dict = interpret_legacy_config(config_dict)
This function would then consecutively update from the config version the config is in to the latest verstion.
| @@ -486,7 +487,9 @@ def parse_spectrum_list2dict(spectrum_list): | |||
| """ | |||
| Parse the spectrum list [start, stop, num] to a list | |||
| """ | |||
|
|
|||
| if isinstance(spectrum_list, dict): | |||
There was a problem hiding this comment.
how about we change to 'if 'start' in spectrum_list and 'stop' in spectrum list and 'num' in spectrum_list`: 😉
There was a problem hiding this comment.
I was expecting a comment on this line.
I even thought about adding a "Dear Wolfgang, this is as temporary as it can get, please act like you didn't notice the missing ducks" comment. 😛
I'll do the change...
| @@ -487,7 +487,8 @@ def parse_spectrum_list2dict(spectrum_list): | |||
| """ | |||
| Parse the spectrum list [start, stop, num] to a list | |||
| """ | |||
| if isinstance(spectrum_list, dict): | |||
| if 'start' in spectrum_list and 'stop' in spectrum_list \ | |||
| and 'num' in spectrum_list: | |||
There was a problem hiding this comment.
This looks complicated, why not try/except?
| @@ -27,7 +27,6 @@ | |||
|
|
|||
| default_config_definition_file = os.path.join(data_dir, | |||
There was a problem hiding this comment.
Do we still need this line?
There was a problem hiding this comment.
No, removed all the references to tardis_config_definition.yml and the file itself.
Note: Tests for the validator are not yet implemented
With this PR I rewrite the
io.config_validatorwhich was a custom-built validator written specifically for TARDIS. With the changes I made, the validator now uses jsonschema, a well-known and broadly-used validation module which validates schemas according to the json-schema specification.For this purpose the configuration definition needed to be transcoded in json schema syntax, which was done at #549.
All currently used types are supported by this validator, as well as setting default values.