Skip to content

Commit

Permalink
Improve validation error message for missing data type (#478)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dsleiter
dsleiter committed Dec 2, 2020
1 parent df96407 commit 2924f4f
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
- Fix inheritance when non-`AbstractContainer` is base class. @rly (#444)
- Fix use of `hdmf.testing.assertContainerEqual(...)` for `Data` objects. @rly (#445)
- Add missing support for data conversion against spec dtypes "bytes" and "short". @rly (#456)
- Clarify the validator error message when a named data type is missing. @dsleiter (#478)

## HDMF 2.2.0 (August 14, 2020)

Expand Down
10 changes: 7 additions & 3 deletions src/hdmf/validate/errors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,15 @@ def __init__(self, **kwargs):
class MissingDataType(Error):
@docval({'name': 'name', 'type': str, 'doc': 'the name of the component that is erroneous'},
{'name': 'data_type', 'type': str, 'doc': 'the missing data type'},
{'name': 'location', 'type': str, 'doc': 'the location of the error', 'default': None})
{'name': 'location', 'type': str, 'doc': 'the location of the error', 'default': None},
{'name': 'missing_dt_name', 'type': str, 'doc': 'the name of the missing data type', 'default': None})
def __init__(self, **kwargs):
name, data_type = getargs('name', 'data_type', kwargs)
name, data_type, missing_dt_name = getargs('name', 'data_type', 'missing_dt_name', kwargs)
self.__data_type = data_type
reason = "missing data type %s" % self.__data_type
if missing_dt_name is not None:
reason = "missing data type %s (%s)" % (self.__data_type, missing_dt_name)
else:
reason = "missing data type %s" % self.__data_type
loc = getargs('location', kwargs)
super().__init__(name, reason, location=loc)

Expand Down
4 changes: 2 additions & 2 deletions src/hdmf/validate/validator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ def validate(self, **kwargs): # noqa: C901
found = True
if not found and self.__include_dts[dt].required:
ret.append(MissingDataType(self.get_spec_loc(self.spec), dt,
location=self.get_builder_loc(builder)))
location=self.get_builder_loc(builder), missing_dt_name=inc_name))
it = chain(self.__dataset_validators.items(),
self.__group_validators.items())
for name, validator in it:
Expand All @@ -472,7 +472,7 @@ def validate(self, **kwargs): # noqa: C901
if sub_builder is None:
if inc_spec.required:
ret.append(MissingDataType(self.get_spec_loc(def_spec), def_spec.data_type_def,
location=self.get_builder_loc(builder)))
location=self.get_builder_loc(builder), missing_dt_name=name))
else:
ret.extend(validator.validate(sub_builder))

Expand Down
72 changes: 45 additions & 27 deletions tests/unit/validator_tests/test_validate.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ def setUp(self):
def getSpecs(self):
pass

def assertValidationError(self, error, type_, name=None, reason=None):
"""Assert that a validation Error matches expectations"""
self.assertIsInstance(error, type_)
if name is not None:
self.assertEqual(error.name, name)
if reason is not None:
self.assertEqual(error.reason, reason)


class TestEmptySpec(ValidatorTestBase):

Expand Down Expand Up @@ -61,29 +69,23 @@ def test_invalid_missing(self):
validator = self.vmap.get_validator('Bar')
result = validator.validate(builder)
self.assertEqual(len(result), 2)
self.assertIsInstance(result[0], MissingError) # noqa: F405
self.assertEqual(result[0].name, 'Bar/attr1')
self.assertIsInstance(result[1], MissingError) # noqa: F405
self.assertEqual(result[1].name, 'Bar/data')
self.assertValidationError(result[0], MissingError, name='Bar/attr1') # noqa: F405
self.assertValidationError(result[1], MissingError, name='Bar/data') # noqa: F405

def test_invalid_incorrect_type_get_validator(self):
builder = GroupBuilder('my_bar', attributes={'data_type': 'Bar', 'attr1': 10})
validator = self.vmap.get_validator('Bar')
result = validator.validate(builder)
self.assertEqual(len(result), 2)
self.assertIsInstance(result[0], DtypeError) # noqa: F405
self.assertEqual(result[0].name, 'Bar/attr1')
self.assertIsInstance(result[1], MissingError) # noqa: F405
self.assertEqual(result[1].name, 'Bar/data')
self.assertValidationError(result[0], DtypeError, name='Bar/attr1') # noqa: F405
self.assertValidationError(result[1], MissingError, name='Bar/data') # noqa: F405

def test_invalid_incorrect_type_validate(self):
builder = GroupBuilder('my_bar', attributes={'data_type': 'Bar', 'attr1': 10})
result = self.vmap.validate(builder)
self.assertEqual(len(result), 2)
self.assertIsInstance(result[0], DtypeError) # noqa: F405
self.assertEqual(result[0].name, 'Bar/attr1')
self.assertIsInstance(result[1], MissingError) # noqa: F405
self.assertEqual(result[1].name, 'Bar/data')
self.assertValidationError(result[0], DtypeError, name='Bar/attr1') # noqa: F405
self.assertValidationError(result[1], MissingError, name='Bar/data') # noqa: F405

def test_valid(self):
builder = GroupBuilder('my_bar',
Expand Down Expand Up @@ -130,8 +132,7 @@ def test_invalid_isodatetime(self):
validator = self.vmap.get_validator('Bar')
result = validator.validate(builder)
self.assertEqual(len(result), 1)
self.assertIsInstance(result[0], DtypeError) # noqa: F405
self.assertEqual(result[0].name, 'Bar/time')
self.assertValidationError(result[0], DtypeError, name='Bar/time') # noqa: F405

def test_invalid_isodatetime_array(self):
builder = GroupBuilder('my_bar',
Expand All @@ -144,36 +145,37 @@ def test_invalid_isodatetime_array(self):
validator = self.vmap.get_validator('Bar')
result = validator.validate(builder)
self.assertEqual(len(result), 1)
self.assertIsInstance(result[0], ExpectedArrayError) # noqa: F405
self.assertEqual(result[0].name, 'Bar/time_array')
self.assertValidationError(result[0], ExpectedArrayError, name='Bar/time_array') # noqa: F405


class TestNestedTypes(ValidatorTestBase):

def getSpecs(self):
baz = DatasetSpec('A dataset with a data type', 'int', data_type_def='Baz',
attributes=[AttributeSpec('attr2', 'an example integer attribute', 'int')])
bar = GroupSpec('A test group specification with a data type',
data_type_def='Bar',
datasets=[DatasetSpec('an example dataset', 'int', name='data',
attributes=[AttributeSpec('attr2', 'an example integer attribute',
'int')])],
datasets=[DatasetSpec('an example dataset', data_type_inc='Baz')],
attributes=[AttributeSpec('attr1', 'an example string attribute', 'text')])
foo = GroupSpec('A test group that contains a data type',
data_type_def='Foo',
groups=[GroupSpec('A Bar group for Foos', name='my_bar', data_type_inc='Bar')],
attributes=[AttributeSpec('foo_attr', 'a string attribute specified as text', 'text',
required=False)])

return (bar, foo)
return (bar, foo, baz)

def test_invalid_missing_req_group(self):
def test_invalid_missing_named_req_group(self):
"""Test that a MissingDataType is returned when a required named nested data type is missing."""
foo_builder = GroupBuilder('my_foo', attributes={'data_type': 'Foo',
'foo_attr': 'example Foo object'})
results = self.vmap.validate(foo_builder)
self.assertIsInstance(results[0], MissingDataType) # noqa: F405
self.assertEqual(results[0].name, 'Foo')
self.assertEqual(results[0].reason, 'missing data type Bar')
self.assertEqual(len(results), 1)
self.assertValidationError(results[0], MissingDataType, name='Foo', # noqa: F405
reason='missing data type Bar (my_bar)')

def test_invalid_wrong_name_req_type(self):
"""Test that a MissingDataType is returned when a required nested data type is given the wrong name."""
bar_builder = GroupBuilder('bad_bar_name',
attributes={'data_type': 'Bar', 'attr1': 'a string attribute'},
datasets=[DatasetBuilder('data', 100, attributes={'attr2': 10})])
Expand All @@ -184,13 +186,28 @@ def test_invalid_wrong_name_req_type(self):

results = self.vmap.validate(foo_builder)
self.assertEqual(len(results), 1)
self.assertIsInstance(results[0], MissingDataType) # noqa: F405
self.assertValidationError(results[0], MissingDataType, name='Foo') # noqa: F405
self.assertEqual(results[0].data_type, 'Bar')

def test_invalid_missing_unnamed_req_group(self):
"""Test that a MissingDataType is returned when a required unnamed nested data type is missing."""
bar_builder = GroupBuilder('my_bar',
attributes={'data_type': 'Bar', 'attr1': 'a string attribute'})

foo_builder = GroupBuilder('my_foo',
attributes={'data_type': 'Foo', 'foo_attr': 'example Foo object'},
groups=[bar_builder])

results = self.vmap.validate(foo_builder)
self.assertEqual(len(results), 1)
self.assertValidationError(results[0], MissingDataType, name='Bar', # noqa: F405
reason='missing data type Baz')

def test_valid(self):
"""Test that no errors are returned when nested data types are correctly built."""
bar_builder = GroupBuilder('my_bar',
attributes={'data_type': 'Bar', 'attr1': 'a string attribute'},
datasets=[DatasetBuilder('data', 100, attributes={'attr2': 10})])
datasets=[DatasetBuilder('data', 100, attributes={'data_type': 'Baz', 'attr2': 10})])

foo_builder = GroupBuilder('my_foo',
attributes={'data_type': 'Foo', 'foo_attr': 'example Foo object'},
Expand All @@ -200,9 +217,10 @@ def test_valid(self):
self.assertEqual(len(results), 0)

def test_valid_wo_opt_attr(self):
""""Test that no errors are returned when an optional attribute is omitted from a group."""
bar_builder = GroupBuilder('my_bar',
attributes={'data_type': 'Bar', 'attr1': 'a string attribute'},
datasets=[DatasetBuilder('data', 100, attributes={'attr2': 10})])
datasets=[DatasetBuilder('data', 100, attributes={'data_type': 'Baz', 'attr2': 10})])
foo_builder = GroupBuilder('my_foo',
attributes={'data_type': 'Foo'},
groups=[bar_builder])
Expand Down

0 comments on commit 2924f4f

Please sign in to comment.