From e022b0cacf4bd921e41cacaa79f695917e67f4b0 Mon Sep 17 00:00:00 2001 From: rly Date: Sat, 10 Aug 2024 01:02:45 -0700 Subject: [PATCH 1/2] Allow override of constructor args and object attrs to return None --- CHANGELOG.md | 1 + src/hdmf/build/objectmapper.py | 61 +++--- .../mapper_tests/test_map_override.py | 177 ++++++++++++++++++ 3 files changed, 209 insertions(+), 30 deletions(-) create mode 100644 tests/unit/build_tests/mapper_tests/test_map_override.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a6369094..262450a17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Enhancements - Added support to append to a dataset of references for HDMF-Zarr. @mavaylon1 [#1157](https://github.com/hdmf-dev/hdmf/pull/1157) +- Allow override of constructor args and object attrs to return None. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167) ## HDMF 3.14.3 (July 29, 2024) diff --git a/src/hdmf/build/objectmapper.py b/src/hdmf/build/objectmapper.py index b5815ee2c..41dd783c8 100644 --- a/src/hdmf/build/objectmapper.py +++ b/src/hdmf/build/objectmapper.py @@ -524,21 +524,21 @@ def map_spec(self, **kwargs): self.map_const_arg(attr_carg, spec) self.map_attr(attr_carg, spec) - def __get_override_carg(self, *args): - name = args[0] - remaining_args = tuple(args[1:]) - if name in self.constructor_args: - self.logger.debug(" Calling override function for constructor argument '%s'" % name) - func = self.constructor_args[name] - return func(self, *remaining_args) - return None + NO_OVERRIDE = object() # non-None sentinel object that signals no override for constructor args and object attrs + + def __get_override_carg(self, argname, builder, manager): + if argname in self.constructor_args: + self.logger.debug(" Calling override function for constructor argument '%s'" % argname) + func = self.constructor_args[argname] + return func(self, builder, manager) + return self.NO_OVERRIDE def __get_override_attr(self, name, container, manager): if name in self.obj_attrs: self.logger.debug(" Calling override function for attribute '%s'" % name) func = self.obj_attrs[name] return func(self, container, manager) - return None + return self.NO_OVERRIDE @docval({"name": "spec", "type": Spec, "doc": "the spec to get the attribute for"}, returns='the attribute name', rtype=str) @@ -558,26 +558,27 @@ def get_attr_value(self, **kwargs): attr_name = self.get_attribute(spec) if attr_name is None: return None - attr_val = self.__get_override_attr(attr_name, container, manager) - if attr_val is None: - try: - attr_val = getattr(container, attr_name) - except AttributeError: - msg = ("%s '%s' does not have attribute '%s' for mapping to spec: %s" - % (container.__class__.__name__, container.name, attr_name, spec)) - raise ContainerConfigurationError(msg) - if isinstance(attr_val, TermSetWrapper): - attr_val = attr_val.value - if attr_val is not None: - attr_val = self.__convert_string(attr_val, spec) - spec_dt = self.__get_data_type(spec) - if spec_dt is not None: - try: - attr_val = self.__filter_by_spec_dt(attr_val, spec_dt, manager) - except ValueError as e: - msg = ("%s '%s' attribute '%s' has unexpected type." - % (container.__class__.__name__, container.name, attr_name)) - raise ContainerConfigurationError(msg) from e + override = self.__get_override_attr(attr_name, container, manager) + if override is not self.NO_OVERRIDE: + return override + try: + attr_val = getattr(container, attr_name) + except AttributeError: + msg = ("%s '%s' does not have attribute '%s' for mapping to spec: %s" + % (container.__class__.__name__, container.name, attr_name, spec)) + raise ContainerConfigurationError(msg) + if isinstance(attr_val, TermSetWrapper): + attr_val = attr_val.value + if attr_val is not None: + attr_val = self.__convert_string(attr_val, spec) + spec_dt = self.__get_data_type(spec) + if spec_dt is not None: + try: + attr_val = self.__filter_by_spec_dt(attr_val, spec_dt, manager) + except ValueError as e: + msg = ("%s '%s' attribute '%s' has unexpected type." + % (container.__class__.__name__, container.name, attr_name)) + raise ContainerConfigurationError(msg) from e # else: attr_val is an attribute on the Container and its value is None # attr_val can be None, an AbstractContainer, or a list of AbstractContainers return attr_val @@ -1334,7 +1335,7 @@ def construct(self, **kwargs): for const_arg in get_docval(cls.__init__): argname = const_arg['name'] override = self.__get_override_carg(argname, builder, manager) - if override is not None: + if override is not self.NO_OVERRIDE: val = override elif argname in const_args: val = const_args[argname] diff --git a/tests/unit/build_tests/mapper_tests/test_map_override.py b/tests/unit/build_tests/mapper_tests/test_map_override.py new file mode 100644 index 000000000..1aeb81d59 --- /dev/null +++ b/tests/unit/build_tests/mapper_tests/test_map_override.py @@ -0,0 +1,177 @@ +from hdmf.build.builders import DatasetBuilder, GroupBuilder +from hdmf.container import Container +from hdmf.spec.spec import AttributeSpec, DatasetSpec, GroupSpec +from hdmf.utils import docval, getargs +from hdmf.build import ObjectMapper +from uuid import uuid4 + +from ...helpers.utils import create_test_type_map, CORE_NAMESPACE + + +class Bar(Container): + @docval( + {"name": "name", "type": str, "doc": "the name of this Foo"}, + {"name": "my_data", "type": ("array_data", "data"), "doc": "some data"}, + {"name": "attr1", "type": str, "doc": "an attribute", "default": None}, + ) + def __init__(self, **kwargs): + name, my_data, attr1 = getargs("name", "my_data", "attr1", kwargs) + super().__init__(name=name) + self.__data = my_data + self.__attr1 = attr1 + + @property + def my_data(self): + return self.__data + + @property + def attr1(self): + return self.__attr1 + + +def test_carg_override(): + """Test that a constructor argument can be overridden by a custom mapper.""" + + class CustomBarMapper(ObjectMapper): + + @ObjectMapper.constructor_arg("attr1") + def attr1_carg(self, builder, manager): + """When constructing a Bar object, use "custom" as the value for the "attr1" argument.""" + return "custom" + + bar_spec = GroupSpec( + "A test group specification with a data type", + data_type_def="Bar", + datasets=[ + DatasetSpec( + name="my_data", + doc="an example 1D int dataset", + dtype="int", + shape=[None], + ) + ], + attributes=[ + AttributeSpec(name="attr1", doc="an example string attribute", dtype="text"), + ], + ) + specs = [bar_spec] + container_classes = {"Bar": Bar} + mappers = {"Bar": CustomBarMapper} + type_map = create_test_type_map(specs, container_classes, mappers) + + bar_builder = GroupBuilder( + name='my_bar', + datasets=[DatasetBuilder(name='my_data', data=[1, 2, 3])], + attributes={'attr1': 'value1', 'namespace': CORE_NAMESPACE, 'object_id': str(uuid4()), 'data_type': 'Bar'} + ) + bar = type_map.construct(bar_builder) + assert bar.attr1 == 'custom' + + +def test_carg_override_none(): + """Test that the constructor_arg method can return None to indicate that the argument should not be set.""" + + class CustomBarMapper(ObjectMapper): + + @ObjectMapper.constructor_arg("attr1") + def attr1_carg(self, builder, manager): + """When constructing a Bar object, use None as the value for the "attr1" argument.""" + return None + + bar_spec = GroupSpec( + "A test group specification with a data type", + data_type_def="Bar", + datasets=[ + DatasetSpec( + name="my_data", + doc="an example 1D int dataset", + dtype="int", + shape=[None], + ) + ], + attributes=[ # attr1 is optional + AttributeSpec(name="attr1", doc="an example string attribute", dtype="text", required=False), + ], + ) + specs = [bar_spec] + container_classes = {"Bar": Bar} + mappers = {"Bar": CustomBarMapper} + type_map = create_test_type_map(specs, container_classes, mappers) + + bar_builder = GroupBuilder( + name='my_bar', + datasets=[DatasetBuilder(name='my_data', data=[1, 2, 3])], + attributes={'attr1': 'value1', 'namespace': CORE_NAMESPACE, 'object_id': str(uuid4()), 'data_type': 'Bar'} + ) + bar = type_map.construct(bar_builder) + assert bar.attr1 is None + + +def test_object_attr_override(): + """Test that an object attribute can be overridden by a custom mapper.""" + + class CustomBarMapper(ObjectMapper): + + @ObjectMapper.object_attr("attr1") + def attr1_attr(self, container, manager): + """When building a Bar object, use "custom" as the value for the "attr1" attribute.""" + return "custom" + + bar_spec = GroupSpec( + "A test group specification with a data type", + data_type_def="Bar", + datasets=[ + DatasetSpec( + name="my_data", + doc="an example 1D int dataset", + dtype="int", + shape=[None], + ) + ], + attributes=[ + AttributeSpec(name="attr1", doc="an example string attribute", dtype="text"), + ], + ) + specs = [bar_spec] + container_classes = {"Bar": Bar} + mappers = {"Bar": CustomBarMapper} + type_map = create_test_type_map(specs, container_classes, mappers) + + bar = Bar(name='my_bar', my_data=[1, 2, 3], attr1='value1') + bar_builder = type_map.build(bar) + assert bar_builder.attributes['attr1'] == 'custom' + + +def test_object_attr_override_none(): + """Test that the object_attr method can return None to indicate that the attribute should not be set.""" + + class CustomBarMapper(ObjectMapper): + + @ObjectMapper.object_attr("attr1") + def attr1_attr(self, container, manager): + """When building a Bar object, use None as the value for the "attr1" attribute.""" + return None + + bar_spec = GroupSpec( + "A test group specification with a data type", + data_type_def="Bar", + datasets=[ + DatasetSpec( + name="my_data", + doc="an example 1D int dataset", + dtype="int", + shape=[None], + ) + ], + attributes=[ # attr1 is optional + AttributeSpec(name="attr1", doc="an example string attribute", dtype="text", required=False), + ], + ) + specs = [bar_spec] + container_classes = {"Bar": Bar} + mappers = {"Bar": CustomBarMapper} + type_map = create_test_type_map(specs, container_classes, mappers) + + bar = Bar(name='my_bar', my_data=[1, 2, 3], attr1='value1') + bar_builder = type_map.build(bar) + assert 'attr1' not in bar_builder.attributes From 69602012a3daa4294b93536a0d8ff9b3edd64232 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 10 Aug 2024 15:02:31 -0700 Subject: [PATCH 2/2] Update CHANGELOG.md Co-authored-by: Oliver Ruebel --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 262450a17..39f0a3add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Enhancements - Added support to append to a dataset of references for HDMF-Zarr. @mavaylon1 [#1157](https://github.com/hdmf-dev/hdmf/pull/1157) -- Allow override of constructor args and object attrs to return None. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167) +- Allow override of constructor args and object attrs in `hdmf.build.ObjectMapper` to return None. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167) ## HDMF 3.14.3 (July 29, 2024)