You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The HDMF validator currently returns a flat list of Error objects from ValidatorMap.validate(). Downstream applications such as PyNWB and NWB Inspector treat any non-empty return list as "this file is invalid".
This shape blocks #624 (and likely future severity levels such as info/hint). We cannot start emitting validation warnings, because mixing warning-level findings into the existing error list would silently flip previously-valid files to invalid the moment any warning fires. Routing warnings through Python's warnings.warn() machinery is also unattractive, since it breaks the validator's "collect and return, let the caller decide" model and forces downstream tools into warnings.catch_warnings() gymnastics just to render warnings alongside errors in the same report.
We need a way for the validator to return both errors and warnings as structured objects, without breaking the large body of existing downstream code that consumes validate().
What solution would you like?
Introduce a small ValidationResult wrapper that carries both lists and behaves like the existing errors list for the dominant usage patterns:
Change ValidatorMap.validate() (and the per-type validators it delegates to) to return ValidationResult instead of a bare list. Add ValidationWarning as a sibling of Error (same .name / .reason / .location shape, not a UserWarning subclass) so warning objects can be rendered uniformly with errors.
Migration properties:
errors = vmap.validate(builder) keeps working.
if errors: is still True only when there are real errors. Warnings do not flip the verdict.
for e in errors: and len(errors) and errors[0] still refer to errors only.
New callers opt in to warnings explicitly via result.warnings.
Only isinstance(x, list) breaks, which is rare.
No coordinated migration is required. PyNWB and NWB Inspector keep working as-is and can opt into surfacing warnings on their own schedule.
Suggested PR breakdown
Plumbing only, no new warnings yet. Add ValidationWarning, add ValidationResult, change the validators to return ValidationResult with warnings always empty. Verify the existing validator test suite passes unchanged.
Actual feature. Add ExtraFieldWarning(ValidationWarning) and the logic from Create validation warnings on extra fields #624 to detect attributes and children that no applicable spec recognizes. Tests assert against result.warnings.
Alternatives considered
New method validate_with_warnings() returning (errors, warnings). Cleaner separation but requires carrying two methods through a long deprecation, with awkward naming where the old validate() is the less capable one.
Kwarg return_warnings=False on validate(). Single method but return type varies with the argument, which is hostile to type checkers and to anyone reading a call site cold.
Severity field on Error with a single mixed list. Requires every downstream caller to update their filtering. Until they do, warnings get treated as invalidating errors. Worst migration story.
warnings.warn() for warnings, errors-only list as today. Breaks the validator's "collect and return" model and complicates joint rendering of errors and warnings.
What would you like to see added to HDMF?
The HDMF validator currently returns a flat list of
Errorobjects fromValidatorMap.validate(). Downstream applications such as PyNWB and NWB Inspector treat any non-empty return list as "this file is invalid".This shape blocks #624 (and likely future severity levels such as info/hint). We cannot start emitting validation warnings, because mixing warning-level findings into the existing error list would silently flip previously-valid files to invalid the moment any warning fires. Routing warnings through Python's
warnings.warn()machinery is also unattractive, since it breaks the validator's "collect and return, let the caller decide" model and forces downstream tools intowarnings.catch_warnings()gymnastics just to render warnings alongside errors in the same report.We need a way for the validator to return both errors and warnings as structured objects, without breaking the large body of existing downstream code that consumes
validate().What solution would you like?
Introduce a small
ValidationResultwrapper that carries both lists and behaves like the existing errors list for the dominant usage patterns:Change
ValidatorMap.validate()(and the per-type validators it delegates to) to returnValidationResultinstead of a bare list. AddValidationWarningas a sibling ofError(same.name/.reason/.locationshape, not aUserWarningsubclass) so warning objects can be rendered uniformly with errors.Migration properties:
errors = vmap.validate(builder)keeps working.if errors:is still True only when there are real errors. Warnings do not flip the verdict.for e in errors:andlen(errors)anderrors[0]still refer to errors only.result.warnings.isinstance(x, list)breaks, which is rare.No coordinated migration is required. PyNWB and NWB Inspector keep working as-is and can opt into surfacing warnings on their own schedule.
Suggested PR breakdown
ValidationWarning, addValidationResult, change the validators to returnValidationResultwithwarningsalways empty. Verify the existing validator test suite passes unchanged.ExtraFieldWarning(ValidationWarning)and the logic from Create validation warnings on extra fields #624 to detect attributes and children that no applicable spec recognizes. Tests assert againstresult.warnings.Alternatives considered
validate_with_warnings()returning(errors, warnings). Cleaner separation but requires carrying two methods through a long deprecation, with awkward naming where the oldvalidate()is the less capable one.return_warnings=Falseonvalidate(). Single method but return type varies with the argument, which is hostile to type checkers and to anyone reading a call site cold.Errorwith a single mixed list. Requires every downstream caller to update their filtering. Until they do, warnings get treated as invalidating errors. Worst migration story.warnings.warn()for warnings, errors-only list as today. Breaks the validator's "collect and return" model and complicates joint rendering of errors and warnings.Context
Do you have any interest in helping implement the feature?
Yes, with guidance for the contributor on #1443.