diff --git a/CHANGELOG.md b/CHANGELOG.md index bf465a4b7..bf12b9b39 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) @@ -116,6 +119,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/common/hdmf-common-schema b/src/hdmf/common/hdmf-common-schema index 4508b8ff8..497bde378 160000 --- a/src/hdmf/common/hdmf-common-schema +++ b/src/hdmf/common/hdmf-common-schema @@ -1 +1 @@ -Subproject commit 4508b8ff8780f4566f4309f144fa7a63f9092cbe +Subproject commit 497bde378749d2e56a30a05d6851885f58e78f75 diff --git a/src/hdmf/validate/errors.py b/src/hdmf/validate/errors.py index c1b4360a1..306f882ae 100644 --- a/src/hdmf/validate/errors.py +++ b/src/hdmf/validate/errors.py @@ -12,11 +12,13 @@ "MissingDataType", "IllegalLinkError", "IncorrectDataType", - "IncorrectQuantityError" + "IncorrectQuantityError", + "ValidationWarning", + "ValidationResult" ] -class Error: +class ValidationIssue: @docval({'name': 'name', 'type': str, 'doc': 'the name of the component that is erroneous'}, {'name': 'reason', 'type': str, 'doc': 'the reason for the error'}, @@ -55,8 +57,11 @@ def __format_str(name, location, reason): def __repr__(self): return self.__str__() + def __eq__(self, other): + return type(self) is type(other) and hash(self) == hash(other) + def __hash__(self): - """Returns the hash value of this Error + """Returns the hash value of this validation issue Note: if the location property is set after creation, the hash value will change. Therefore, it is important to finalize the value of location @@ -81,8 +86,37 @@ def __equatable_str(self): equatable_name = self.name return self.__format_str(equatable_name, self.location, self.reason) - def __eq__(self, other): - return hash(self) == hash(other) + +class Error(ValidationIssue): + """A validation error""" + pass + + +class ValidationWarning(ValidationIssue): + """A validation warning""" + pass + + +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): 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..2adfb3f26 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,53 @@ 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 == [] + + def test_error_and_warning_with_same_attributes_are_not_equal(self): + """Test that an Error and a ValidationWarning with the same name, reason, and location are not equal. """ + err = Error(name="TestIssue", reason="Same issue", location="root") + warn = ValidationWarning(name="TestIssue", reason="Same issue", location="root") + + self.assertNotEqual(err, warn)