Refactor validator return type to support warnings via ValidationResult wrapper (#1479)#1480
Refactor validator return type to support warnings via ValidationResult wrapper (#1479)#1480sejalpunwatkar wants to merge 3 commits into
Conversation
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1480 +/- ##
==========================================
- Coverage 93.18% 93.04% -0.15%
==========================================
Files 41 41
Lines 10176 10225 +49
Branches 2103 2105 +2
==========================================
+ Hits 9483 9514 +31
- Misses 415 433 +18
Partials 278 278 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| def __init__(self, errors = None, warnings = None): | ||
| self.errors = list(errors) if errors is not None else [] | ||
| self.warnings = list(warnings) if errors is not None else [] |
There was a problem hiding this comment.
| self.warnings = list(warnings) if errors is not None else [] | |
| self.warnings = list(warnings) if warnings is not None else [] |
| "IllegalLinkError", | ||
| "IncorrectDataType", | ||
| "IncorrectQuantityError" | ||
| ] |
There was a problem hiding this comment.
Please add the new classes to __all__
|
|
||
| @docval({'name': 'builder', 'type': BaseBuilder, 'doc': 'the builder to validate'}, | ||
| returns="a list of errors found", rtype=list) | ||
| returns="a list of errors found", rtype=(list, ValidationResult)) |
There was a problem hiding this comment.
| returns="a list of errors found", rtype=(list, ValidationResult)) | |
| returns="A ValidationResult containing the errors and warnings found", rtype=ValidationResult) |
There was a problem hiding this comment.
Please also update the docstring of validate(...) in src/hdmf/common/__init__.py:210
|
Thanks! Looks good overall. Could you please add some basic tests of
Could you please also add a changelog entry that points to this PR? |
| def __getitem__(self, i): | ||
| return self.errors[i] |
There was a problem hiding this comment.
Below this, could you add a simple __repr__ so that users can print this object (like they did the list before) and get a human-readable listing of the errors and warnings instead of the object address?
2 participants
Motivation
Addresses #1479
This PR implements Phase 1 (the foundational plumbing) to introduce
ValidationWarninginfrastructure without breaking the existing error reporting model. This change ensures that downstream packages (like PyNWB and NWB Inspector) can eventually consume warning-level messages without modifying their immediate logic or incorrectly marking files as invalid.How to test the behavior?
ValidationWarningclass as a sibling toErrorinsideerrors.py, preserving the matching internal shape (.name,.reason,.location).ValidationResultwrapper class inerrors.pywith custom Python overrides (__bool__,__len__,__iter__,__getitem__) pointing explicitly to theerrorsbucket to ensure backward compatibility.ValidatorMap.validate()insidevalidator.pyto capture theerrors_list, wrap it into the newValidationResultcontainer with an empty warnings container (warnings=[]), and adjusted its strict@docvalreturn type check tortype=(list, ValidationResult).Checklist
CHANGELOG.mdwith your changes?