Add ValidationReport class for validation results#2165
Add ValidationReport class for validation results#2165sejalpunwatkar wants to merge 4 commits intoNeurodataWithoutBorders:devfrom
Conversation
This change introduces a new ValidationReport dataclass to provide a structured representation of NWB validation results. Instead of returning raw validation errors alone, validation now returns report objects containing contextual metadata while preserving existing validation behavior.
| ] | ||
|
|
||
| @dataclass | ||
| class ValidationReport: |
There was a problem hiding this comment.
Could you please add a docstring for the ValidationReport calss and its functions/parameters
There was a problem hiding this comment.
Thanks for the suggestion! I’ll add docstrings for the ValidationReport class and its methods/parameters.
There was a problem hiding this comment.
Pull request overview
Introduces a ValidationReport dataclass intended to provide a structured representation of NWB validation results (including metadata like path/namespace/timestamp) rather than returning only raw validation errors.
Changes:
- Added a new
ValidationReportdataclass and exported it via__all__. - Updated validation flow to construct and return
ValidationReportobjects per namespace validated. - Updated the
validatedocval return description to refer to validation “reports”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/pynwb/validation.py
Outdated
| return validation_errors | ||
| io.close() | ||
|
|
||
| return validation_reports |
There was a problem hiding this comment.
_validate_single_file now returns a list of ValidationReport objects, which changes the public validate() return value from a flat list of error strings to report objects. This is a breaking API change and also breaks internal consumers that assume a list of strings (e.g., src/pynwb/testing/testh5io.py uses "\n".join(errors) and src/pynwb/validation_cli.py treats len(val_errors)>0 as “has errors”). To preserve existing behavior, keep validate() returning the original flat list of error strings by default and introduce reports behind a new API (e.g., validate(..., return_reports=True) or a separate validate_with_report() function), or otherwise update all in-repo call sites and deprecation notes accordingly.
| errors: list | ||
| created_at: str = field(default_factory=lambda: datetime.utcnow().isoformat()) | ||
|
|
There was a problem hiding this comment.
ValidationReport uses errors: list and created_at: str with datetime.utcnow().isoformat(), which produces a timezone-naive timestamp string and loses type information for consumers. Consider using precise typing for errors (e.g., List[str] if these are error strings) and storing created_at as a timezone-aware datetime (or, if you need a string, use datetime.now(timezone.utc).isoformat() and include the UTC offset).
Fix validation error aggregation in validate().
Change validation report to return validation errors
|
Hi @oruebel, |
This change introduces a new ValidationReport dataclass to provide a structured representation of NWB validation results. Instead of returning raw validation errors alone, validation now returns report objects containing contextual metadata while preserving existing validation behavior.