diff --git a/CHANGELOG.md b/CHANGELOG.md index 8693a8029..01d7d70fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## 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) + ### Documentation and tutorial enhancements - Expanded the "Read HERD" section of the external resources tutorial to show how to inspect a `HERD` after reading it back with `HERD.from_zip`, using `to_dataframe`, the individual interlinked tables, and `get_object_entities`. This addresses confusion about a read `HERD` appearing empty in its default Jupyter display. @rly [#1535](https://github.com/hdmf-dev/hdmf/pull/1535) @@ -113,6 +116,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) diff --git a/src/hdmf/common/__init__.py b/src/hdmf/common/__init__.py index 20d39905a..c386a0ff1 100644 --- a/src/hdmf/common/__init__.py +++ b/src/hdmf/common/__init__.py @@ -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 @@ -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, is_method=False) def validate(**kwargs): - """Validate an file against a namespace""" + """Validate a file against a namespace.""" + io, namespace, experimental = getargs('io', 'namespace', 'experimental', kwargs) if experimental: namespace = EXP_NAMESPACE diff --git a/src/hdmf/validate/errors.py b/src/hdmf/validate/errors.py index c1b4360a1..3fb8ea05d 100644 --- a/src/hdmf/validate/errors.py +++ b/src/hdmf/validate/errors.py @@ -12,7 +12,9 @@ "MissingDataType", "IllegalLinkError", "IncorrectDataType", - "IncorrectQuantityError" + "IncorrectQuantityError", + "ValidationWarning", + "ValidationResult" ] @@ -85,6 +87,81 @@ def __eq__(self, other): return hash(self) == hash(other) +class ValidationWarning: + + @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] + + 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'}, diff --git a/src/hdmf/validate/validator.py b/src/hdmf/validate/validator.py index c7709e80b..d80e148ed 100644 --- a/src/hdmf/validate/validator.py +++ b/src/hdmf/validate/validator.py @@ -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 @@ -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 @@ -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): diff --git a/tests/unit/validator_tests/test_validate.py b/tests/unit/validator_tests/test_validate.py index 0717ef401..e5296c181 100644 --- a/tests/unit/validator_tests/test_validate.py +++ b/tests/unit/validator_tests/test_validate.py @@ -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' @@ -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 == []