Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# HDMF Changelog

## HDMF 6.2.0 (Upcoming)

### Enhancements
- Refactored validator return type to `ValidationResult` to support upcoming validation warnings. @sejalpunwatkar [#1480](https://github.com/hdmf-dev/hdmf/pull/1480)


## HDMF 6.1.0 (June 25, 2026)

### Enhancements
Expand Down Expand Up @@ -91,6 +97,7 @@
- Removed `test-min-deps` dependency group and replaced it with `uv pip install --resolution lowest-direct` in tox, making the project compatible with uv. @h-mayorquin [#1408](https://github.com/hdmf-dev/hdmf/pull/1408)
- Changed `get_data_shape` to check `shape` before `maxshape`, so that objects with both attributes (e.g., h5py datasets) return their actual shape rather than their maximum shape. @rly [#1180](https://github.com/hdmf-dev/hdmf/pull/1180)


### Removed
- Dropped support for Python 3.9. The minimum supported version is now Python 3.10. @rly [#xxx](https://github.com/hdmf-dev/hdmf/pull/xxx)
- Replaced `typing` library calls with Python 3.10+ built-in type syntax (`X | Y`, `X | None`, `type[X]`, `tuple[X]`, etc.). @rly [#xxx](https://github.com/hdmf-dev/hdmf/pull/xxx)
Expand Down
7 changes: 4 additions & 3 deletions src/hdmf/common/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from ..utils import docval, getargs, get_docval, AllowPositional # noqa: E402
from ..backends.io import HDMFIO # noqa: E402
from ..backends.hdf5 import HDF5IO # noqa: E402
from ..validate import ValidatorMap # noqa: E402
from ..validate import ValidatorMap, ValidationResult # noqa: E402
from ..build import BuildManager, TypeMap # noqa: E402
from ..container import _set_exp # noqa: E402

Expand Down Expand Up @@ -207,10 +207,11 @@ def get_manager(**kwargs):
'doc': 'the namespace to validate against', 'default': CORE_NAMESPACE},
{'name': 'experimental', 'type': bool,
'doc': 'data type is an experimental data type', 'default': False},
returns="errors in the file", rtype=list,
returns="errors and warnings in the file", rtype=ValidationResult,
Comment thread
sejalpunwatkar marked this conversation as resolved.
is_method=False)
def validate(**kwargs):
"""Validate an file against a namespace"""
"""Validate an file against a namespace."""

io, namespace, experimental = getargs('io', 'namespace', 'experimental', kwargs)
if experimental:
namespace = EXP_NAMESPACE
Expand Down
79 changes: 78 additions & 1 deletion src/hdmf/validate/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
"MissingDataType",
"IllegalLinkError",
"IncorrectDataType",
"IncorrectQuantityError"
"IncorrectQuantityError",
"ValidationWarning",
"ValidationResult"
]


Expand Down Expand Up @@ -85,6 +87,81 @@ def __eq__(self, other):
return hash(self) == hash(other)


class ValidationWarning:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ValidationWarning is almost entirely identical to Error, it would be better to create a shared base class ValidationIssue that both extend from and that contains all the shared code. Downstream code will still be able to differentiate between Error and ValidationWarning because they are different classes.

One minor thing is that because __eq__ is defined based on the hash and I believe the hash of a ValidationWarning and the hash of an identical Error object will be the same, then Error("name", "reason") == ValidationWarning("name", "reason") will be True. This might become an issue if/when errors and warnings are placed in a dict together.

To address this, I would change __eq__ to:

    def __eq__(self, other):
          return type(self) is type(other) and hash(self) == hash(other)


@docval({'name': 'name', 'type': str, 'doc': 'the name of the component that is erroneous'},
{'name': 'reason', 'type': str, 'doc': 'the reason for the warning'},
{'name': 'location', 'type': str, 'doc': 'the location of the warning', 'default': None})
def __init__(self, **kwargs):
self.__name = getargs('name', kwargs)
self.__reason = getargs('reason', kwargs)
self.__location = getargs('location', kwargs)

@property
def name(self):
return self.__name

@property
def reason(self):
return self.__reason

@property
def location(self):
return self.__location

@location.setter
def location(self, loc):
self.__location = loc

def __str__(self):
return self.__format_str(self.name, self.location, self.reason)

@staticmethod
def __format_str(name, location, reason):
if location is not None:
return "%s (%s): %s" % (name, location, reason)
else:
return "%s: %s" % (name, reason)

def __repr__(self):
return self.__str__()

def __hash__(self):
return hash(self.__equatable_str())

def __equatable_str(self):
if self.location is not None:
equatable_name = self.name.split('/')[-1]
else:
equatable_name = self.name
return self.__format_str(equatable_name, self.location, self.reason)

def __eq__(self, other):
return type(self) is type(other) and hash(self) == hash(other)


class ValidationResult:

def __init__(self, errors = None, warnings = None):
self.errors = list(errors) if errors is not None else []
self.warnings = list(warnings) if warnings is not None else []

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]
Comment thread
rly marked this conversation as resolved.

def __repr__(self):
return "ValidationResult(errors=%r, warnings=%r)" % (self.errors, self.warnings)


class DtypeError(Error):

@docval({'name': 'name', 'type': str, 'doc': 'the name of the component that is erroneous'},
Expand Down
7 changes: 4 additions & 3 deletions src/hdmf/validate/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import numpy as np

from .errors import Error, DtypeError, MissingError, MissingDataType, ShapeError, IllegalLinkError, IncorrectDataType
from .errors import ExpectedArrayError, IncorrectQuantityError
from .errors import ExpectedArrayError, IncorrectQuantityError, ValidationResult
from ..build import GroupBuilder, DatasetBuilder, LinkBuilder, ReferenceBuilder
from ..build.builders import BaseBuilder
from ..spec import Spec, AttributeSpec, GroupSpec, DatasetSpec, RefSpec, LinkSpec
Expand Down Expand Up @@ -325,7 +325,7 @@ def get_validator(self, **kwargs):
raise ValueError(msg)

@docval({'name': 'builder', 'type': BaseBuilder, 'doc': 'the builder to validate'},
returns="a list of errors found", rtype=list)
returns="A ValidationResult containing the errors and warnings found", rtype=ValidationResult)
def validate(self, **kwargs):
"""Validate a builder against a Spec

Expand All @@ -338,7 +338,8 @@ def validate(self, **kwargs):
msg = "builder must have data type defined with attribute '%s'" % self.__type_key
raise ValueError(msg)
validator = self.get_validator(dt)
return validator.validate(builder)
errors_list = validator.validate(builder)
return ValidationResult(errors=errors_list, warnings=[])


class Validator(metaclass=ABCMeta):
Expand Down
47 changes: 45 additions & 2 deletions tests/unit/validator_tests/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
from hdmf.testing import TestCase, remove_test_file
from hdmf.validate import ValidatorMap
from hdmf.validate.errors import (DtypeError, MissingError, ExpectedArrayError, MissingDataType,
IncorrectQuantityError, IllegalLinkError, ShapeError, IncorrectDataType)
IncorrectQuantityError, IllegalLinkError, ShapeError, IncorrectDataType,
ValidationWarning, ValidationResult, Error)
from hdmf.backends.hdf5 import HDF5IO
from hdmf.utils import ZARR_INSTALLED, StrDataset
from hdmf.validate.errors import Error

CORE_NAMESPACE = 'test_core'

Expand Down Expand Up @@ -2043,3 +2043,46 @@ def test_isodatetime_no_time_component_fails(self):
# This confirms it fails because it lacks the 'T' and timezone
self.assertEqual(len(result), 1)
self.assertIsInstance(result[0], Error)


class TestValidationResultWrapper(TestCase):
"""Unit tests for the ValidationResult container and ValidationWarning class.

These tests verify that the ValidationResult wrapper correctly isolates
warnings while maintaining perfect backward compatibility by ensuring that
magic methods (__len__, __bool__, __iter__, __getitem__) reflect the errors
list only.
"""

def test_validation_result_basic_behavior(self):
err = Error(name="TestError", reason="Critical issue", location="root")
warn = ValidationWarning(name="TestWarning", reason="Minor issue", location="root")

result = ValidationResult(errors=[err], warnings=[warn])
self.assertEqual(result.errors, [err])
self.assertEqual(result.warnings, [warn])
self.assertEqual(len(result), 1)
self.assertTrue(bool(result))
self.assertEqual(result[0], err)
self.assertEqual(list(result), [err])

def test_validation_result_empty_behavior(self):
empty_result = ValidationResult()
assert len(empty_result) == 0
assert bool(empty_result) is False
assert empty_result.warnings == []

def test_validate_method_returns_empty_warnings(self):
"""Test that the validate method returns a ValidationResult with empty warnings for clean data."""

catalog = SpecCatalog()
catalog.register_spec(GroupSpec('A dummy spec', data_type_def='Dummy'), 'test.yaml')
namespace = SpecNamespace('test ns', 'test_ns', [{'source': 'test.yaml'}], version='0.1.0', catalog=catalog)
vmap = ValidatorMap(namespace)

builder = GroupBuilder('root', attributes={'data_type': 'Dummy'})

result = vmap.validate(builder)

assert isinstance(result, ValidationResult)
assert result.warnings == []
Loading