-
Notifications
You must be signed in to change notification settings - Fork 30
Validation changes #389
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
Validation changes #389
Conversation
The Section repository present validation is no longer part of the default validation and should be added on demand.
A validation report is now printed as a warnings module UserWarnings if a saved or loaded Document contains Validation warnings or errors. On load the warnings will only be printed when the Parsers 'show_warnings' attribute is True.
|
Looks great! Thanks. One small note: I think the reason I brought up 3-digit status codes in our last meeting was to separate the categories by the first digit, so |
|
That was quick. @jgrewe I'm good to merge. |
This PR contains a couple of potentially controversial points and should be discussed here before merging and addresses the issues #377, #378 and #379.
Changes to the Validation class enabling Custom Validation Instances sneaked itself in at an earlier commit and should be taken into account when reviewing this PR. Check changes in
Validation.__init__()andrun_validationandregister_custom_handlermethods. Sorry for the mixup...This PR
ValidationError.__repr__to make it more convenient to read.reportmethod to theValidationclass to provide a general overview of collected errors and warnings.reportmethod whenever a Document is saved or loaded via theODMLParserresulting in awarnings.warnmessage:object_name_readablevalidation message toName not assignedto make it clearer to the user that an object name should be set.section_repository_presentvalidation from the default validation list. Since Sections rarely have repositories set, this validation can get a bit spammy when validating a Document.Fixes:
SectionandPropertyis added: when trying to set thenameattribute toNone, it now silently sets the name toidinstead, sincenamemust not be empty. It would be set toidon load and can causeAttributeErrorexceptions with some methods if its not set.Points for discussion before merging:
saveandloadas it is now or make any changes to it?ODMLParser,odml.loadandodml.save, not when using the underlyingXMLParserfor XML orDictParserfor YAML and JSON to keep the validations out of the core parsers. Any different opinions on this point?