Simplify schema handling and fix several bugs #35
Conversation
Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
- Loads schema files from within the package. - Uses utility functions instead of duplicating validation code. - Deprecates old utility functions. Signed-off-by: John Pennycook <[email protected]>
Loading from file highlighted that we had extra commas. Signed-off-by: John Pennycook <[email protected]>
Validating the JSON object directly (without first dumping and reloading a JSON string) requires us to follow the schema more strictly. Previously, the call to json.dumps() was converting tuples to lists for us. Signed-off-by: John Pennycook <[email protected]>
codebasin/util.py
Outdated
| @@ -141,6 +113,90 @@ def valid_path(path): | |||
| return valid | |||
|
|
|||
|
|
|||
| def _validate_json(json_object: str, schema_name: str) -> bool: | |||
There was a problem hiding this comment.
This is overly pedantic and doesn't matter because it's not so public facing, but this function is JSON centric but it's not actually; config.py feeds the output of a YAML file into this, so you might want to rename the function more generically to be like _validate_config, and pass a config: dict[str, Any]
There was a problem hiding this comment.
I can see where you're coming from.
I'm hoping that we'll be able to remove the YAML configuration files completely in 2.0.0... Do you think we should rename the function for now (to reflect that it's occasionally used for JSON and YAML), but change the name back when we remove the YAML?
There was a problem hiding this comment.
Ok, I decided to split the difference: see 7256a84.
We can keep the existing _validate_json function, and remove the new _validate_yaml function when we no longer need it.
Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
This is more or less the same fix as was applied in intel/p3-analysis-library#26, but with some quirks specific to Code Base Investigator's packaging. Using the schema as written in the
*.schemafiles uncovered a few bugs that were hidden by using preconfigured Python objects, so I fixed those too.One thing I want to call attention to is my handling of the
util.validate_coverage_jsonfunction and its replacements. I've kept it around (but deprecated it) because I don't want this refactor to be what pushes us to 2.0.0. The replacements are all declared as private functions, though, because I think 2.0.0 should be more intentional in what we expose as part of the package -- there's no reason to expose our utility functions directly.