-
Notifications
You must be signed in to change notification settings - Fork 6
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
Validate properties that refer to other datamodels #1008
Conversation
… into validate-multiple-instances
Co-authored-by: Francesca L. Bleken <[email protected]>
… into validate-multiple-instances
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.
Nice PR. Added some comments
If you have time for it, it would be nice with some tests for this PR as well. |
First, I think that we should never get a segfault from Python. That is a bug that should be fixed independently. If there is an error in the dimensions an exception should be raised. Regarding your question, dlite should hopefully raise an exception if there is an error. These are already handled, so I don't think we need to do additional error handling in dlite-validate. However, if there is an error in the datamodel or instance that is not detected by dlite, we could either handle it here or update the underlying dlite framework to raise a proper exception. I think the choice depends on the nature of the error and its severity. |
Co-authored-by: Francesca L. Bleken <[email protected]>
In principle I agree, but the setup and the placements of tests and how to run them is a bit complicated. I have added some new valid and invalid datamodels that can be tested. Note that the InvalidYaml2.yaml is the one that causes the segmentation fault. It happens when nothing is given the dimensions. |
PR #1010 should fix the segfault |
See PR #1011 for how to add tests with CMake. |
# Description Avoid that dlite segfaults if "dimensions" is null (empty) in JSON and YAML input. ## Type of change - [x] Bug fix & code cleanup - [ ] New feature - [ ] Documentation update - [ ] Test update ## Checklist for the reviewer This checklist should be used as a help for the reviewer. - [ ] Is the change limited to one issue? - [ ] Does this PR close the issue? - [ ] Is the code easy to read and understand? - [ ] Do all new feature have an accompanying new test? - [ ] Has the documentation been updated as necessary?
# Description Added tests for dlite-validate of instances with ref-types. Merge PR #1010 first to get rid of the segfaults. ## Type of change - [ ] Bug fix & code cleanup - [ ] New feature - [ ] Documentation update - [x] Test update ## Checklist for the reviewer This checklist should be used as a help for the reviewer. - [ ] Is the change limited to one issue? - [ ] Does this PR close the issue? - [ ] Is the code easy to read and understand? - [ ] Do all new feature have an accompanying new test? - [ ] Has the documentation been updated as necessary?
This pull request includes changes to the
dlite-validate
file to improve the validation of datamodels that refer to other datamodels in their properties.Question: Should we add error-handling here?
Problem that I do not know how to fix: If there is and error in the dimension in the references datamodel I get a segmentation fault and the whole process closes down without error message.
builds on #986
closes #1007
Type of change
Checklist for the reviewer
This checklist should be used as a help for the reviewer.
This pull request includes changes to the
bindings/python/scripts/dlite-validate
file to improve the validation process for instances and their properties. The most important changes include adding validation for property references and updating the import and spacing for better readability.