Allow override of constructor args and object attrs to return None

CHANGELOG.md

@@ -4,17 +4,18 @@
 
 ### 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 in `hdmf.build.ObjectMapper` to return None. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167)
 - Adjusted stacklevel of warnings to point to user code when possible. @rly [#1166](https://github.com/hdmf-dev/hdmf/pull/1166)
 - Improved "already exists" error message when adding a container to a `MultiContainerInterface`. @rly [#1165](https://github.com/hdmf-dev/hdmf/pull/1165)
+- Speed up loading namespaces by skipping register_type when already registered. @magland [#1102](https://github.com/hdmf-dev/hdmf/pull/1102)
+- Speed up namespace loading: return a shallow copy rather than a deep copy in build_const_args. @magland [#1103](https://github.com/hdmf-dev/hdmf/pull/1103)
 
 ## HDMF 3.14.3 (July 29, 2024)
 
 ### Enhancements
 - Added new attribute "dimension_labels" on `DatasetBuilder` which specifies the names of the dimensions used in the
 dataset based on the shape of the dataset data and the dimension names in the spec for the data type. This attribute
 is available on build (during the write process), but not on read of a dataset from a file. @rly [#1081](https://github.com/hdmf-dev/hdmf/pull/1081)
-- Speed up loading namespaces by skipping register_type when already registered. @magland [#1102](https://github.com/hdmf-dev/hdmf/pull/1102)
-- Speed up namespace loading: return a shallow copy rather than a deep copy in build_const_args. @magland [#1103](https://github.com/hdmf-dev/hdmf/pull/1103)
 
 ## HDMF 3.14.2 (July 7, 2024)
 

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]

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