Skip to content

Commit

Permalink
Validation finds unexpected elements (#542)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsleiter authored Apr 21, 2021
1 parent 2205e1f commit 67cffec
Show file tree
Hide file tree
Showing 10 changed files with 500 additions and 100 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
@oruebel (#551)
- Add tutoral for new `AlignedDynamicTable` type. @oruebel (#571)
- Equality check for `DynamicTable` now also checks that the name and description of the table are the same. @rly (#566)
- Add `ExtraFieldWarning` validator warning, which is created when fields that are not part of the spec are
encountered. @dsleiter, @t-b, @rly (#542)

### Internal improvements
- Update CI and copyright year. @rly (#523, #524)
Expand Down
6 changes: 3 additions & 3 deletions src/hdmf/build/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ def get_builder_ns(self, **kwargs):
builder = getargs('builder', kwargs)
if isinstance(builder, LinkBuilder):
builder = builder.builder
ret = builder.attributes.get('namespace')
ret = builder.attributes.get(self.namespace_catalog.group_spec_cls.namespace_key())
return ret

@docval({'name': 'builder', 'type': Builder,
Expand Down Expand Up @@ -713,7 +713,7 @@ def register_container_type(self, **kwargs):
self.__data_types.setdefault(container_cls, (namespace, data_type))
if not isinstance(container_cls, TypeSource):
setattr(container_cls, spec.type_key(), data_type)
setattr(container_cls, 'namespace', namespace)
setattr(container_cls, spec.namespace_key(), namespace)

@docval({"name": "container_cls", "type": type,
"doc": "the AbstractContainer class for which the given ObjectMapper class gets used for"},
Expand Down Expand Up @@ -751,7 +751,7 @@ def build(self, **kwargs):

# add additional attributes (namespace, data_type, object_id) to builder
namespace, data_type = self.get_container_ns_dt(container)
builder.set_attribute('namespace', namespace)
builder.set_attribute(obj_mapper.spec.namespace_key(), namespace)
builder.set_attribute(self.__type_key(obj_mapper.spec), data_type)
builder.set_attribute(obj_mapper.spec.id_key(), container.object_id)
return builder
Expand Down
17 changes: 16 additions & 1 deletion src/hdmf/spec/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ class BaseStorageSpec(Spec):

__inc_key = 'data_type_inc'
__def_key = 'data_type_def'
__namespace_key = 'namespace'
__type_key = 'data_type'
__id_key = 'object_id'

Expand Down Expand Up @@ -437,13 +438,27 @@ def get_data_type_spec(cls, data_type_def):

@classmethod
def get_namespace_spec(cls):
return AttributeSpec('namespace', 'the namespace for the data type of this object', 'text', required=False)
return AttributeSpec(cls.namespace_key(), 'the namespace for the data type of this object',
'text', required=False)

@property
def attributes(self):
''' The attributes for this specification '''
return tuple(self.get('attributes', tuple()))

@classmethod
def reserved_attrs(cls):
''' Get the names of the reserved attributes '''
return (cls.namespace_key(), cls.type_key(), cls.id_key())

@classmethod
def namespace_key(cls):
''' Get the key used to store namespace on an instance
Override this method to use a different name for 'namespace'
'''
return cls.__namespace_key

@property
def linkable(self):
''' True if object can be a link, False otherwise '''
Expand Down
25 changes: 19 additions & 6 deletions src/hdmf/testing/testcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
import re
import unittest
from abc import ABCMeta, abstractmethod
from warnings import warn

from .utils import remove_test_file
from ..backends.hdf5 import HDF5IO
from ..build import Builder
from ..common import validate as common_validate, get_manager
from ..container import AbstractContainer, Container, Data
from ..query import HDMFDataset
from ..validate.errors import WARNING_SEVERITY


class TestCase(unittest.TestCase):
Expand Down Expand Up @@ -225,14 +227,25 @@ def validate(self, experimental=False):
"""Validate the written and exported files, if they exist."""
if os.path.exists(self.filename):
with HDF5IO(self.filename, manager=get_manager(), mode='r') as io:
errors = common_validate(io, experimental=experimental)
result = common_validate(io, experimental=experimental)
errors, warnings = self._split_validation_result(result)
if errors:
for err in errors:
raise Exception(err)
raise Exception("Validation Errors %s" % errors)
if warnings:
warn("Validation Warnings: %s" % warnings)

if os.path.exists(self.export_filename):
with HDF5IO(self.filename, manager=get_manager(), mode='r') as io:
errors = common_validate(io, experimental=experimental)
result = common_validate(io, experimental=experimental)
errors, warnings = self._split_validation_result(result)
if errors:
for err in errors:
raise Exception(err)
raise Exception("Validation Errors %s" % errors)
if warnings:
warn("Validation Warnings: %s" % warnings)

@staticmethod
def _split_validation_result(result):
"""Split validation result into errors and warnings"""
errors = [alert for alert in result if alert.severity > WARNING_SEVERITY]
warnings = [alert for alert in result if alert.severity == WARNING_SEVERITY]
return errors, warnings
22 changes: 21 additions & 1 deletion src/hdmf/validate/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,25 @@
"MissingDataType",
"IllegalLinkError",
"IncorrectDataType",
"IncorrectQuantityError"
"IncorrectQuantityError",
"ExtraFieldWarning"
]

WARNING_SEVERITY = 0
ERROR_SEVERITY = 10


class Error:

@docval({'name': 'name', 'type': str, 'doc': 'the name of the component that is erroneous'},
{'name': 'reason', 'type': str, 'doc': 'the reason for the error'},
{'name': 'severity', 'type': int, 'doc': 'severity of the error', 'default': ERROR_SEVERITY},
{'name': 'location', 'type': str, 'doc': 'the location of the error', 'default': None})
def __init__(self, **kwargs):
self.__name = getargs('name', kwargs)
self.__reason = getargs('reason', kwargs)
self.__location = getargs('location', kwargs)
self.__severity = getargs('severity', kwargs)
if self.__location is not None:
self.__str = "%s (%s): %s" % (self.__name, self.__location, self.__reason)
else:
Expand All @@ -42,6 +48,10 @@ def reason(self):
def location(self):
return self.__location

@property
def severity(self):
return self.__severity

@location.setter
def location(self, loc):
self.__location = loc
Expand Down Expand Up @@ -176,3 +186,13 @@ def __init__(self, **kwargs):
reason = "incorrect data_type - expected '%s', got '%s'" % (expected, received)
loc = getargs('location', kwargs)
super().__init__(name, reason, location=loc)


class ExtraFieldWarning(Error):
@docval({'name': 'name', 'type': str, 'doc': 'the name of the component with the extra field'},
{'name': 'location', 'type': str, 'doc': 'the location of the warning', 'default': None})
def __init__(self, **kwargs):
name = getargs('name', kwargs)
reason = "encountered extra field"
loc = getargs('location', kwargs)
super().__init__(name, reason, location=loc, severity=WARNING_SEVERITY)
Loading

0 comments on commit 67cffec

Please sign in to comment.