Skip to content

Remove the None-return fall-through in ObjectMapper override functions (HDMF 8.0) #1541

Description

@rly

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:

  1. 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.
  2. 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).
  3. 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.
  4. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: lowalternative solution already working and/or relevant to only specific user(s)

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions