From 39050bb08a00ccb590ae6f51f01880e6d4b05765 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Tue, 26 May 2020 11:11:35 -0700 Subject: [PATCH] Improve `get_class` and `docval` support for uint (#361) --- CHANGELOG.md | 7 ++- src/hdmf/build/manager.py | 52 +++++++++++++++++---- src/hdmf/spec/spec.py | 2 + src/hdmf/utils.py | 10 +++- tests/unit/build_tests/test_io_map.py | 23 ++++++++++ tests/unit/utils_test/test_docval.py | 66 ++++++++++++++++++++++++++- 6 files changed, 146 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6b243d94..9c68aedee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) @@ -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) diff --git a/src/hdmf/build/manager.py b/src/hdmf/build/manager.py index cdb451370..a22b5d380 100644 --- a/src/hdmf/build/manager.py +++ b/src/hdmf/build/manager.py @@ -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): @@ -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): @@ -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): @@ -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): diff --git a/src/hdmf/spec/spec.py b/src/hdmf/spec/spec.py index bf4252b3f..359a01736 100644 --- a/src/hdmf/spec/spec.py +++ b/src/hdmf/spec/spec.py @@ -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"], diff --git a/src/hdmf/utils.py b/src/hdmf/utils.py index 448384b7b..c0dd0e2af 100644 --- a/src/hdmf/utils.py +++ b/src/hdmf/utils.py @@ -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 @@ -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): @@ -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': @@ -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) diff --git a/tests/unit/build_tests/test_io_map.py b/tests/unit/build_tests/test_io_map.py index 01d88d2c6..de4a28785 100644 --- a/tests/unit/build_tests/test_io_map.py +++ b/tests/unit/build_tests/test_io_map.py @@ -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): diff --git a/tests/unit/utils_test/test_docval.py b/tests/unit/utils_test/test_docval.py index eba35db2a..bb0cca731 100644 --- a/tests/unit/utils_test/test_docval.py +++ b/tests/unit/utils_test/test_docval.py @@ -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..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..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): @@ -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..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, )}) @@ -602,7 +662,8 @@ 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) @@ -610,9 +671,10 @@ def method(self, **kwargs): 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..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)