Skip to content

Commit

Permalink
Improve get_class and docval support for uint (#361)
Browse files Browse the repository at this point in the history
  • Loading branch information
rly authored May 26, 2020
1 parent 1b5fb35 commit 39050bb
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 14 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# HDMF Changelog

## HDMF 1.6.2 (May 18, 2020)
## HDMF 1.6.2 (May 26, 2020)

### Internal improvements:
- Update MacOS in CI. @rly (#310)
Expand All @@ -17,12 +17,17 @@
- Add logging of build and hdf5 write process. @rly (#336, #349)
- Allow loading namespaces from h5py.File object not backed by file. @rly (#348)
- Add CHANGELOG.md. @rly (#352)
- Fix codecov reports. @rly (#362)
- Make `getargs` raise an error if the argument name is not found. @rly (#365)
- Improve `get_class` and `docval` support for uint. @rly (#361)

### Bug fixes:
- Register new child types before new parent type for dynamic class generation. @rly (#322)
- Raise warning not error when adding column with existing attr name. @rly (#324)
- Add `__version__`. @rly (#345)
- Only write a specific namespace version if it does not exist. @ajtritt (#346)
- Fix documentation formatting for DynamicTable. @rly (#353)


## HDMF 1.6.1 (Mar. 2, 2020)

Expand Down
52 changes: 42 additions & 10 deletions src/hdmf/build/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,16 +377,40 @@ def load_namespaces(self, **kwargs):
self.register_container_type(new_ns, dt, container_cls)
return deps

_type_map = {
# mapping from spec types to allowable python types for docval for fields during dynamic class generation
# e.g., if a dataset/attribute spec has dtype int32, then get_class should generate a docval for the class'
# __init__ method that allows the types (int, np.int32, np.int64) for the corresponding field.
# passing an np.int16 would raise a docval error.
# passing an int64 to __init__ would result in the field storing the value as an int64 (and subsequently written
# as an int64). no upconversion or downconversion happens as a result of this map
# see https://schema-language.readthedocs.io/en/latest/specification_language_description.html#dtype
_spec_dtype_map = {
'float32': (float, np.float32, np.float64),
'float': (float, np.float32, np.float64),
'float64': (float, np.float64),
'double': (float, np.float64),
'int8': (np.int8, np.int16, np.int32, np.int64, int),
'int16': (np.int16, np.int32, np.int64, int),
'short': (np.int16, np.int32, np.int64, int),
'int32': (int, np.int32, np.int64),
'int': (int, np.int32, np.int64),
'int64': np.int64,
'long': np.int64,
'uint8': (np.uint8, np.uint16, np.uint32, np.uint64),
'uint16': (np.uint16, np.uint32, np.uint64),
'uint32': (np.uint32, np.uint64),
'uint64': np.uint64,
'numeric': (float, np.float32, np.float64, np.int8, np.int16, np.int32, np.int64, int, np.uint8, np.uint16,
np.uint32, np.uint64),
'text': str,
'float': float,
'float32': float,
'float64': float,
'int': int,
'int32': int,
'utf': str,
'utf8': str,
'utf-8': str,
'ascii': bytes,
'bytes': bytes,
'bool': bool,
'uint64': np.uint64,
'isodatetime': datetime
'isodatetime': datetime,
'datetime': datetime
}

def __get_container_type(self, container_name):
Expand All @@ -399,6 +423,14 @@ def __get_container_type(self, container_name):
# this code should never happen after hdmf#322
raise TypeDoesNotExistError("Type '%s' does not exist." % container_name)

def __get_scalar_type_map(self, spec_dtype):
dtype = self._spec_dtype_map.get(spec_dtype)
if dtype is None: # pragma: no cover
# this should not happen as long as _spec_dtype_map is kept up to date with
# hdmf.spec.spec.DtypeHelper.valid_primary_dtypes
raise ValueError("Spec dtype '%s' cannot be mapped to a Python type." % spec_dtype)
return dtype

def __get_type(self, spec):
if isinstance(spec, AttributeSpec):
if isinstance(spec.dtype, RefSpec):
Expand All @@ -409,7 +441,7 @@ def __get_type(self, spec):
return container_type
return Data, Container
elif spec.shape is None and spec.dims is None:
return self._type_map.get(spec.dtype)
return self.__get_scalar_type_map(spec.dtype)
else:
return 'array_data', 'data'
if isinstance(spec, LinkSpec):
Expand All @@ -419,7 +451,7 @@ def __get_type(self, spec):
if spec.data_type_inc is not None:
return self.__get_container_type(spec.data_type_inc)
if spec.shape is None and spec.dims is None:
return self._type_map.get(spec.dtype)
return self.__get_scalar_type_map(spec.dtype)
return 'array_data', 'data'

def __ischild(self, dtype):
Expand Down
2 changes: 2 additions & 0 deletions src/hdmf/spec/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

class DtypeHelper():
# Dict where the keys are the primary data type and the values are list of strings with synonyms for the dtype
# this is also used in the validator
# if this list is updated, also update hdmf.build.manager.TypeMap._spec_dtype_map
primary_dtype_synonyms = {
'float': ["float", "float32"],
'double': ["double", "float64"],
Expand Down
10 changes: 9 additions & 1 deletion src/hdmf/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
AllowPositional = Enum('AllowPositional', 'ALLOWED WARNING ERROR')

__supported_bool_types = (bool, np.bool_)
__supported_uint_types = (np.uint8, np.uint16, np.uint32, np.uint64)
__supported_int_types = (int, np.int8, np.int16, np.int32, np.int64)
__supported_float_types = [float, np.float16, np.float32, np.float64]
if hasattr(np, "float128"): # pragma: no cover
Expand All @@ -24,7 +25,8 @@
# non-deterministically. a future version of h5py will fix this. see #112
__supported_float_types.append(np.longdouble)
__supported_float_types = tuple(__supported_float_types)
__allowed_enum_types = __supported_bool_types + __supported_int_types + __supported_float_types + (str, )
__allowed_enum_types = (__supported_bool_types + __supported_uint_types + __supported_int_types
+ __supported_float_types + (str, ))


def docval_macro(macro):
Expand Down Expand Up @@ -59,6 +61,8 @@ def __type_okay(value, argtype, allow_none=False):
if isinstance(argtype, str):
if argtype in __macros:
return __type_okay(value, __macros[argtype], allow_none=allow_none)
elif argtype == 'uint':
return __is_uint(value)
elif argtype == 'int':
return __is_int(value)
elif argtype == 'float':
Expand Down Expand Up @@ -97,6 +101,10 @@ def __shape_okay(value, argshape):
return True


def __is_uint(value):
return isinstance(value, __supported_uint_types)


def __is_int(value):
return isinstance(value, __supported_int_types)

Expand Down
23 changes: 23 additions & 0 deletions tests/unit/build_tests/test_io_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,29 @@ def test_dynamic_container_composition_missing_type(self):
with self.assertRaisesWith(ValueError, msg):
self.manager.type_map.get_container_cls(CORE_NAMESPACE, 'Baz1')

def test_dynamic_container_uint(self):
baz_spec = GroupSpec('A test extension with no Container class',
data_type_def='Baz', data_type_inc=self.bar_spec,
attributes=[AttributeSpec('attr3', 'an example uint16 attribute', 'uint16'),
AttributeSpec('attr4', 'another example float attribute', 'float')])
self.spec_catalog.register_spec(baz_spec, 'extension.yaml')
cls = self.type_map.get_container_cls(CORE_NAMESPACE, 'Baz')
for arg in get_docval(cls.__init__):
if arg['name'] == 'attr3':
self.assertTupleEqual(arg['type'], (np.uint16, np.uint32, np.uint64))

def test_dynamic_container_numeric(self):
baz_spec = GroupSpec('A test extension with no Container class',
data_type_def='Baz', data_type_inc=self.bar_spec,
attributes=[AttributeSpec('attr3', 'an example numeric attribute', 'numeric'),
AttributeSpec('attr4', 'another example float attribute', 'float')])
self.spec_catalog.register_spec(baz_spec, 'extension.yaml')
cls = self.type_map.get_container_cls(CORE_NAMESPACE, 'Baz')
for arg in get_docval(cls.__init__):
if arg['name'] == 'attr3':
self.assertTupleEqual(arg['type'], (float, np.float32, np.float64, np.int8, np.int16, np.int32,
np.int64, int, np.uint8, np.uint16, np.uint32, np.uint64))


class ObjectMapperMixin(metaclass=ABCMeta):

Expand Down
66 changes: 64 additions & 2 deletions tests/unit/utils_test/test_docval.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,52 @@ def method(self, **kwargs):
self.assertEqual(res, np.bool_(True))
self.assertIsInstance(res, np.bool_)

def test_uint_type(self):
"""Test that docval type specification of np.uint32 works as expected."""
@docval({'name': 'arg1', 'type': np.uint32, 'doc': 'this is a uint'})
def method(self, **kwargs):
return popargs('arg1', kwargs)

res = method(self, arg1=np.uint32(1))
self.assertEqual(res, np.uint32(1))
self.assertIsInstance(res, np.uint32)

msg = ("TestDocValidator.test_uint_type.<locals>.method: incorrect type for 'arg1' (got 'uint8', expected "
"'uint32')")
with self.assertRaisesWith(TypeError, msg):
method(self, arg1=np.uint8(1))

msg = ("TestDocValidator.test_uint_type.<locals>.method: incorrect type for 'arg1' (got 'uint64', expected "
"'uint32')")
with self.assertRaisesWith(TypeError, msg):
method(self, arg1=np.uint64(1))

def test_uint_string_type(self):
"""Test that docval type specification of string 'uint' matches np.uint of all available precisions."""
@docval({'name': 'arg1', 'type': 'uint', 'doc': 'this is a uint'})
def method(self, **kwargs):
return popargs('arg1', kwargs)

res = method(self, arg1=np.uint(1))
self.assertEqual(res, np.uint(1))
self.assertIsInstance(res, np.uint)

res = method(self, arg1=np.uint8(1))
self.assertEqual(res, np.uint8(1))
self.assertIsInstance(res, np.uint8)

res = method(self, arg1=np.uint16(1))
self.assertEqual(res, np.uint16(1))
self.assertIsInstance(res, np.uint16)

res = method(self, arg1=np.uint32(1))
self.assertEqual(res, np.uint32(1))
self.assertIsInstance(res, np.uint32)

res = method(self, arg1=np.uint64(1))
self.assertEqual(res, np.uint64(1))
self.assertIsInstance(res, np.uint64)

def test_allow_positional_warn(self):
@docval({'name': 'arg1', 'type': bool, 'doc': 'this is a bool'}, allow_positional=AllowPositional.WARNING)
def method(self, **kwargs):
Expand Down Expand Up @@ -587,6 +633,20 @@ def method(self, **kwargs):
with self.assertRaisesWith(ValueError, msg):
method(self, 3)

def test_enum_uint(self):
"""Test that the basic usage of an enum check on uints works"""
@docval({'name': 'arg1', 'type': np.uint, 'doc': 'an arg', 'enum': (np.uint(1), np.uint(2))})
def method(self, **kwargs):
return popargs('arg1', kwargs)

self.assertEqual(method(self, np.uint(1)), np.uint(1))
self.assertEqual(method(self, np.uint(2)), np.uint(2))

msg = ("TestDocValidator.test_enum_uint.<locals>.method: "
"forbidden value for 'arg1' (got 3, expected (1, 2))")
with self.assertRaisesWith(ValueError, msg):
method(self, np.uint(3))

def test_enum_float(self):
"""Test that the basic usage of an enum check on floats works"""
@docval({'name': 'arg1', 'type': float, 'doc': 'an arg', 'enum': (3.14, )})
Expand All @@ -602,17 +662,19 @@ def method(self, **kwargs):

def test_enum_bool_mixed(self):
"""Test that the basic usage of an enum check on a tuple of bool, int, float, and string works"""
@docval({'name': 'arg1', 'type': (bool, int, float, str), 'doc': 'an arg', 'enum': (True, 1, 1.0, 'true')})
@docval({'name': 'arg1', 'type': (bool, int, float, str, np.uint), 'doc': 'an arg',
'enum': (True, 1, 1.0, 'true', np.uint(1))})
def method(self, **kwargs):
return popargs('arg1', kwargs)

self.assertEqual(method(self, True), True)
self.assertEqual(method(self, 1), 1)
self.assertEqual(method(self, 1.0), 1.0)
self.assertEqual(method(self, 'true'), 'true')
self.assertEqual(method(self, np.uint(1)), np.uint(1))

msg = ("TestDocValidator.test_enum_bool_mixed.<locals>.method: "
"forbidden value for 'arg1' (got 0, expected (True, 1, 1.0, 'true'))")
"forbidden value for 'arg1' (got 0, expected (True, 1, 1.0, 'true', 1))")
with self.assertRaisesWith(ValueError, msg):
method(self, 0)

Expand Down

0 comments on commit 39050bb

Please sign in to comment.