Skip to content

Refactor validator return type to support warnings as well as errors #1479

Description

@rly

What would you like to see added to HDMF?

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:

class ValidationResult:
    def __init__(self, errors, warnings):
        self.errors = errors
        self.warnings = warnings
    def __iter__(self):       return iter(self.errors)
    def __len__(self):        return len(self.errors)
    def __bool__(self):       return bool(self.errors)
    def __getitem__(self, i): return self.errors[i]

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

  1. 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.
  2. 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.

Context

Do you have any interest in helping implement the feature?

Yes, with guidance for the contributor on #1443.

Metadata

Metadata

Assignees

No one assigned

    Labels

    topic: PyNWBIssues related to the use HDMF in PyNWBtopic: validatorissues related to validation of files

    Fields

    No fields configured for Enhancement.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions