hayfield
commented
6 years ago Suggest add information about other exceptions that could be raised (such as occurred an malformed XML file)
If other exceptions are raised here, that sounds like an implementation bug - a minimal failing test case should be created and a fix made. Any lxml exception that appears is a bug and should not be part of the module's interface.
the docstring should make it clearer that this function checks against codelists.
And Rulesets. Adding something like given its population state to the summary line provides the high-level information. More detail could then be added to the rest of the description.
Other than the difference in return types, what is the difference between full_validation and is_valid ??
They should be undertaking the same steps such that the is_valid() implementation could be not full_validation(dataset, schema).contains_errors().
I'd suggest that this might have been missed in the conversion to _check_x()-style functions.
Indeed, what does ‘full validation’ even mean?
The intention is along the lines of "check ALL THE THINGS and give me details about errors".
The private / public split of functions returning alternative levels of detail about specific things should be looked at - eg. there's nothing in the module's public interface to only perform Codelist validation at the moment.
After working with @allthatilk to use validation functionality, we have the following comments regarding the naming and documentation of validation functions:
is_iati_xml- Suggest add information about other exceptions that could be raised (such as occurred an malformed XML file)full_validationis_iati_xmlis_validis_valid, the docstring should make it clearer that this function checks against codelists.full_validationandis_valid??