From 657d3e1afe0389173d1e593f9e7564372eff6105 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:40:07 -0700 Subject: [PATCH 1/3] Make HERDManager an abstract interface, update __gather_fields - Make HERDManager an ABC with abstract external_resources property. Subclasses must declare external_resources in their __fields__ (or __nwbfields__) to satisfy the interface and get auto-generated getter/setter. - Update __gather_fields to override inherited abstract properties with auto-generated ones and clear __abstractmethods__ accordingly. - Remove _gather_mixin_fields (no longer needed since subclasses declare fields themselves). - Update all HERDManager subclasses to declare external_resources in their __fields__. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/gallery/plot_external_resources.py | 5 +++ src/hdmf/container.py | 47 +++++++++++++++++++------ src/hdmf/testing/make_test_files.py | 4 ++- tests/unit/common/test_resources.py | 5 +++ tests/unit/helpers/utils.py | 4 +++ tests/unit/test_container.py | 36 +++++++++++++++---- tests/unit/test_io_hdf5_h5tools.py | 4 ++- 7 files changed, 86 insertions(+), 19 deletions(-) diff --git a/docs/gallery/plot_external_resources.py b/docs/gallery/plot_external_resources.py index 45272980a..b8490dc62 100644 --- a/docs/gallery/plot_external_resources.py +++ b/docs/gallery/plot_external_resources.py @@ -109,6 +109,11 @@ # Class to represent a file class HERDManagerContainer(Container, HERDManager): + + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) + def __init__(self, **kwargs): kwargs['name'] = 'HERDManagerContainer' super().__init__(**kwargs) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index fd11b1518..b4f6031bf 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -1,4 +1,5 @@ import types +from abc import ABC, abstractmethod from collections import OrderedDict from copy import deepcopy from uuid import uuid4 @@ -31,20 +32,37 @@ def _exp_warn_msg(cls): return msg -class HERDManager: - """ - When this class is used as a mixin for a Container, it enables setting and getting an instance of HERD. +class HERDManager(ABC): + """An abstract mixin interface for containers that support external resources. + + Container subclasses that inherit from this mixin must declare + ``external_resources`` in their ``__fields__`` (or equivalent, e.g., + ``__nwbfields__``) with ``'child': True`` so that the auto-generated + property satisfies the abstract interface. The subclass must also create + an ObjectMapper mapping so that the ``external_resources`` value is + written to a file on write and populated from a file on read. + + Example:: + + class MyFile(HERDManager, Container): + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) + + This class serves as a marker for ``isinstance`` checks throughout + hdmf (e.g., in HERD and HDMFIO) to identify containers that support + external resources. """ @property + @abstractmethod def external_resources(self): - return self._herd if hasattr(self, "_herd") else None + pass @external_resources.setter - def external_resources(self, herd): - if hasattr(self, "_herd"): - warn("Reassigning external_resources may lead to unexpected behavior.") - self._herd = herd + @abstractmethod + def external_resources(self, val): + pass class AbstractContainer(metaclass=ExtenderMeta): @@ -256,15 +274,24 @@ def __gather_fields(cls, name, bases, classdict): base_fields_conf_to_add.append(pconf) all_fields_conf[0:0] = base_fields_conf_to_add - # create getter and setter if attribute does not already exist + # create getter and setter if attribute does not already exist or is abstract # if 'doc' not specified in __fields__, use doc from docval of __init__ docs = {dv['name']: dv['doc'] for dv in get_docval(cls.__init__)} + abstracts_overridden = set() for field_conf in all_fields_conf: pname = field_conf['name'] field_conf.setdefault('doc', docs.get(pname)) - if not hasattr(cls, pname): + existing = getattr(cls, pname, None) + if existing is None or getattr(existing, '__isabstractmethod__', False): + if getattr(existing, '__isabstractmethod__', False): + abstracts_overridden.add(pname) setattr(cls, pname, property(cls._getter(field_conf), cls._setter(field_conf))) + # update __abstractmethods__ to remove any that were satisfied by auto-generated properties + if abstracts_overridden: + remaining = getattr(cls, '__abstractmethods__', frozenset()) - abstracts_overridden + cls.__abstractmethods__ = remaining + cls._set_fields(tuple(field_conf['name'] for field_conf in all_fields_conf)) cls.__fieldsconf = tuple(all_fields_conf) diff --git a/src/hdmf/testing/make_test_files.py b/src/hdmf/testing/make_test_files.py index f41c22ca0..1698406a0 100644 --- a/src/hdmf/testing/make_test_files.py +++ b/src/hdmf/testing/make_test_files.py @@ -10,7 +10,9 @@ class _HERDManagerContainer(SimpleMultiContainer, HERDManager): """A SimpleMultiContainer that also implements HERDManager, for testing.""" - pass + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) def make_herd_1_8_0_file(outdir): diff --git a/tests/unit/common/test_resources.py b/tests/unit/common/test_resources.py index 3dbb71bcd..df6a5fb87 100644 --- a/tests/unit/common/test_resources.py +++ b/tests/unit/common/test_resources.py @@ -21,6 +21,11 @@ class HERDManagerContainer(Container, HERDManager): + + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) + def __init__(self, **kwargs): kwargs['name'] = 'HERDManagerContainer' super().__init__(**kwargs) diff --git a/tests/unit/helpers/utils.py b/tests/unit/helpers/utils.py index d9f9d18fe..53267cd26 100644 --- a/tests/unit/helpers/utils.py +++ b/tests/unit/helpers/utils.py @@ -124,6 +124,10 @@ class FooFile(Container, HERDManager): and should be reset to 'root' when use is finished to avoid potential cross-talk between tests. """ + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) + ROOT_NAME = "root" # For HDF5 and Zarr this is the root. It should be set before use if different for the backend. @docval( diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index ab3845a24..872712f96 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -26,17 +26,39 @@ def __init__(self, **kwargs): self.field1 = kwargs['field1'] +class ContainerWithHERD(HERDManager, Container): + """A test Container subclass that uses the HERDManager mixin.""" + + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) + + class TestHERDManager(TestCase): - def test_get_and_set_resources(self): - em = HERDManager() - er = HERD() + def test_subclass_has_external_resources_field(self): + """Test that a subclass declaring external_resources in __fields__ has it.""" + self.assertIn('external_resources', ContainerWithHERD.__fields__) - em.external_resources = er - self.assertEqual(em.external_resources, er) + def test_mixin_external_resources_default_none(self): + """Test that external_resources defaults to None when not set.""" + container = ContainerWithHERD(name='test') + self.assertIsNone(container.external_resources) - er_get = em.external_resources - self.assertEqual(er, er_get) + def test_mixin_set_external_resources(self): + """Test setting external_resources on a Container subclass with HERDManager.""" + container = ContainerWithHERD(name='test') + er = HERD() + container.external_resources = er + self.assertIs(container.external_resources, er) + + def test_mixin_external_resources_is_child(self): + """Test that external_resources is registered as a child of the container.""" + container = ContainerWithHERD(name='test') + er = HERD() + container.external_resources = er + self.assertIn(er, container.children) + self.assertIs(er.parent, container) class TestContainer(TestCase): diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index 8628aa261..f47f4e641 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -1244,7 +1244,9 @@ def test_write_herd_as_child(self): link from HERD to the data can be verified.""" class _HERDManagerContainer(SimpleMultiContainer, HERDManager): - pass + __fields__ = ( + {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'}, + ) species = Data(name='species', data=['Homo sapiens']) herd = HERD() From cea011f2f2f88ecfaa324045c698c1bb4e7cdc23 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Mon, 23 Mar 2026 14:50:35 -0700 Subject: [PATCH 2/3] Add changelog entry for HERDManager abstract interface (#1431) Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ede8d922..a71156874 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # HDMF Changelog +## HDMF Unreleased + +### Enhancements +- Made `HERDManager` an abstract interface (ABC) with an abstract `external_resources` property. Subclasses must now declare `external_resources` in their `__fields__` to satisfy the interface. Updated `__gather_fields` to allow auto-generated properties to override inherited abstract properties. @rly [#1431](https://github.com/hdmf-dev/hdmf/pull/1431) + ## HDMF 5.0.1 (March 16, 2026) ### Enhancements From e115bce6cc9556f3f7a8eac2bf143c92d0459ed7 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:30:06 -0700 Subject: [PATCH 3/3] Add link_resources and get_external_resources to HERDManager Move linked HERD functionality from NWBFile to HERDManager base class so any HERDManager subclass can manage a secondary linked HERD alongside the primary one. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 2 +- src/hdmf/container.py | 27 +++++++++++++++++++++++++++ tests/unit/test_container.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71156874..e43350e9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## HDMF Unreleased ### Enhancements -- Made `HERDManager` an abstract interface (ABC) with an abstract `external_resources` property. Subclasses must now declare `external_resources` in their `__fields__` to satisfy the interface. Updated `__gather_fields` to allow auto-generated properties to override inherited abstract properties. @rly [#1431](https://github.com/hdmf-dev/hdmf/pull/1431) +- Made `HERDManager` an abstract interface (ABC) with an abstract `external_resources` property. Subclasses must now declare `external_resources` in their `__fields__` to satisfy the interface. Updated `__gather_fields` to allow auto-generated properties to override inherited abstract properties. Added `link_resources` and `get_external_resources` methods to `HERDManager` for managing a linked HERD separate from the primary one. @rly [#1431](https://github.com/hdmf-dev/hdmf/pull/1431) ## HDMF 5.0.1 (March 16, 2026) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index b4f6031bf..11b190b24 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -54,6 +54,10 @@ class MyFile(HERDManager, Container): external resources. """ + def __init__(self, **kwargs): + super().__init__(**kwargs) + self._linked_external_resources = None + @property @abstractmethod def external_resources(self): @@ -64,6 +68,29 @@ def external_resources(self): def external_resources(self, val): pass + def link_resources(self, external_resources): + """Link an external HERD object as the external resources for this container. + + The linked HERD will not be written on export; the original HERD + (if any) is preserved in the exported file. Use + ``get_external_resources(linked=True)`` to access the linked HERD. + """ + self._linked_external_resources = external_resources + + def get_external_resources(self, linked=False): + """Return the HERD external resources object for this container. + + Parameters + ---------- + linked : bool, optional + If True, return the linked HERD set via ``link_resources``. + If False (default), return the HERD set via ``__init__`` or the + ``external_resources`` attribute. + """ + if linked: + return self._linked_external_resources + return self.external_resources + class AbstractContainer(metaclass=ExtenderMeta): # The name of the class attribute that subclasses use to autogenerate properties diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index 872712f96..9db2a67cf 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -60,6 +60,36 @@ def test_mixin_external_resources_is_child(self): self.assertIn(er, container.children) self.assertIs(er.parent, container) + def test_link_resources(self): + """Test linking an external HERD object.""" + container = ContainerWithHERD(name='test') + linked_herd = HERD() + container.link_resources(linked_herd) + self.assertIs(container.get_external_resources(linked=True), linked_herd) + + def test_get_external_resources_default(self): + """Test get_external_resources returns the primary HERD by default.""" + container = ContainerWithHERD(name='test') + er = HERD() + container.external_resources = er + self.assertIs(container.get_external_resources(), er) + self.assertIs(container.get_external_resources(linked=False), er) + + def test_get_external_resources_linked_default_none(self): + """Test get_external_resources(linked=True) returns None when no linked HERD is set.""" + container = ContainerWithHERD(name='test') + self.assertIsNone(container.get_external_resources(linked=True)) + + def test_link_resources_does_not_affect_primary(self): + """Test that linking a HERD does not overwrite the primary external_resources.""" + container = ContainerWithHERD(name='test') + primary = HERD() + linked = HERD() + container.external_resources = primary + container.link_resources(linked) + self.assertIs(container.get_external_resources(), primary) + self.assertIs(container.get_external_resources(linked=True), linked) + class TestContainer(TestCase):