What would you like changed or added and why?
Follow-up to #1167, which added ObjectMapper.NO_OVERRIDE and deprecated the practice of returning None from a constructor_arg or object_attr override function to mean "no override".
As of #1167, an override function that returns None still falls through to the value built from the file (in construct) or read from the container attribute (in get_attr_value), but emits a DeprecationWarning when that fall-through actually changes the result. Override functions should return ObjectMapper.NO_OVERRIDE to fall through silently.
In HDMF 8.0, the deprecation period ends and a None return should set the constructor argument or attribute to None.
Do you have any interest in helping implement the feature?
Yes.
Changes required
Both sites are marked with TODO(HDMF 8.0) comments in src/hdmf/build/objectmapper.py:
ObjectMapper.construct (around the __get_override_carg call): remove the if override is None: branch that emits the DeprecationWarning and reassigns override = self.NO_OVERRIDE. A None return should flow straight through to kwargs[argname] = None.
ObjectMapper.get_attr_value (around the __get_override_attr call): change the guard from if override is not self.NO_OVERRIDE and override is not None: to if override is not self.NO_OVERRIDE:, and remove the DeprecationWarning block below it. Only NO_OVERRIDE should fall through to getattr(container, attr_name).
- Update the deprecation tests in
tests/unit/build_tests/mapper_tests/test_map_override.py (the tests currently asserting the DeprecationWarning and the fall-through result) to assert the new behavior: the argument/attribute is set to None.
- Update
docs/source/extensions.rst (or wherever override functions are documented) and CHANGELOG.md.
Downstream impact
Any override function in PyNWB or an extension that returns None to signal "no override" will silently start setting the value to None after this change. Before releasing 8.0, PyNWB and the extensions we know about should be re-audited for constructor_arg/object_attr functions with a bare return None or an implicit None return path, and updated to return ObjectMapper.NO_OVERRIDE. The DeprecationWarning added in #1167 is what gives downstream time to do this.
What would you like changed or added and why?
Follow-up to #1167, which added
ObjectMapper.NO_OVERRIDEand deprecated the practice of returningNonefrom aconstructor_argorobject_attroverride function to mean "no override".As of #1167, an override function that returns
Nonestill falls through to the value built from the file (inconstruct) or read from the container attribute (inget_attr_value), but emits aDeprecationWarningwhen that fall-through actually changes the result. Override functions should returnObjectMapper.NO_OVERRIDEto fall through silently.In HDMF 8.0, the deprecation period ends and a
Nonereturn should set the constructor argument or attribute toNone.Do you have any interest in helping implement the feature?
Yes.
Changes required
Both sites are marked with
TODO(HDMF 8.0)comments insrc/hdmf/build/objectmapper.py:ObjectMapper.construct(around the__get_override_cargcall): remove theif override is None:branch that emits theDeprecationWarningand reassignsoverride = self.NO_OVERRIDE. ANonereturn should flow straight through tokwargs[argname] = None.ObjectMapper.get_attr_value(around the__get_override_attrcall): change the guard fromif override is not self.NO_OVERRIDE and override is not None:toif override is not self.NO_OVERRIDE:, and remove theDeprecationWarningblock below it. OnlyNO_OVERRIDEshould fall through togetattr(container, attr_name).tests/unit/build_tests/mapper_tests/test_map_override.py(the tests currently asserting theDeprecationWarningand the fall-through result) to assert the new behavior: the argument/attribute is set toNone.docs/source/extensions.rst(or wherever override functions are documented) andCHANGELOG.md.Downstream impact
Any override function in PyNWB or an extension that returns
Noneto signal "no override" will silently start setting the value toNoneafter this change. Before releasing 8.0, PyNWB and the extensions we know about should be re-audited forconstructor_arg/object_attrfunctions with a barereturn Noneor an implicitNonereturn path, and updated to returnObjectMapper.NO_OVERRIDE. TheDeprecationWarningadded in #1167 is what gives downstream time to do this.