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

Add schema validation for json data #167

Merged
merged 14 commits into from
Aug 9, 2023
Merged

Add schema validation for json data #167

merged 14 commits into from
Aug 9, 2023

Conversation

CunliangGeng
Copy link
Member

Fix #165.

Major changes:

  • added json schemas and related utils to new folder src/nplinker/schemas
  • added unit tests on json schema itself and relevant valid/invalid data to new folder tests/schemas
  • added json data validation after loading json data or before saving data to file, i.e. following the load-validate and validate-save patterns
  • update some code and test data to match the requirements of json schema

@CunliangGeng CunliangGeng added this to the refactor codebase milestone Aug 8, 2023
@CunliangGeng CunliangGeng self-assigned this Aug 8, 2023
@CunliangGeng CunliangGeng linked an issue Aug 8, 2023 that may be closed by this pull request
Copy link
Contributor

@gcroci2 gcroci2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good! jsonschema has very nice functionalities.

The only comment I have is that three schema tests (for GENOME_BGC_MAPPINGS_SCHEMA, GENOME_STATUS_SCHEMA, and STRAIN_MAPPINGS_SCHEMA) have some redundancies (e.g., 'version' is a required property, data_invalid_version, ). We could think about a way to iter over the three schemas and reduce the number of files (and lines of code), but I am not sure if this is worth it, and it's not going to be easy. On one hand indeed, it's useful for a developer to have the three distinct files for testing the three schemas, it makes things very clear.

@CunliangGeng
Copy link
Member Author

We should keep the tests on different schemas separate, in this way the tests are modular and clear, most importantly, it's easier to extend when schema changes.

@CunliangGeng CunliangGeng merged commit e1973da into dev Aug 9, 2023
2 of 5 checks passed
@CunliangGeng CunliangGeng deleted the add_json_validation branch August 9, 2023 13:47
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.

Add schema validation
2 participants