Skip to content

Make HERDManager an abstract interface#1431

Merged
rly merged 3 commits into
devfrom
herd-manager-mixin-improvements
Mar 24, 2026
Merged

Make HERDManager an abstract interface#1431
rly merged 3 commits into
devfrom
herd-manager-mixin-improvements

Conversation

@rly

@rly rly commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Motivation

HERDManager was previously a concrete mixin that used a private _herd attribute with a manual property, bypassing the __fields__ / child-tracking system entirely. This PR refactors it into an abstract interface (ABC) that:

  1. Defines an abstract external_resources property, requiring subclasses to provide the implementation
  2. Subclasses satisfy the interface by declaring external_resources in their __fields__ (or __nwbfields__), which triggers auto-generated getter/setter via __gather_fields
  3. Serves as a marker for isinstance checks in HERD and HDMFIO

This design is cleaner because:

  • The mixin doesn't dictate field configuration — subclasses can set required_name, child, etc. as needed
  • external_resources can now be a proper child field with parent tracking in container subclasses like NWBFile

__gather_fields is updated to allow auto-generated properties to override inherited abstract properties, clearing __abstractmethods__ accordingly.

Additionally, link_resources and get_external_resources methods are added to HERDManager so that any subclass can manage a secondary linked HERD alongside the primary one. This moves functionality that was previously NWBFile-specific into the base class for broader reuse.

This PR will make the corresponding PyNWB PR NeurodataWithoutBorders/pynwb#2111 cleaner.

How to test the behavior?

from hdmf.container import Container, HERDManager
from hdmf.common.resources import HERD

class MyFile(HERDManager, Container):
    __fields__ = (
        {'name': 'external_resources', 'child': True, 'required_name': 'external_resources'},
    )

f = MyFile(name='test')
f.external_resources = HERD()
assert f.external_resources.parent is f
assert isinstance(f, HERDManager)

# Linked HERD support
linked = HERD()
f.link_resources(linked)
assert f.get_external_resources() is f.external_resources  # primary
assert f.get_external_resources(linked=True) is linked      # linked

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Does the PR clearly describe the problem and the solution?
  • Have you reviewed our Contributing Guide?
  • Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

rly and others added 2 commits March 23, 2026 14:34
- 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) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Mar 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.13%. Comparing base (c7dcb5f) to head (e115bce).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1431      +/-   ##
==========================================
+ Coverage   93.10%   93.13%   +0.02%     
==========================================
  Files          41       41              
  Lines       10038    10047       +9     
  Branches     2061     2063       +2     
==========================================
+ Hits         9346     9357      +11     
+ Misses        416      415       -1     
+ Partials      276      275       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rly

rly commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Optional tests, pynwb tests, and hdmf-zarr tests are expected to fail until the PyNWB 4 release.

@rly rly requested review from oruebel March 23, 2026 22:49
oruebel
oruebel previously approved these changes Mar 23, 2026
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) <noreply@anthropic.com>
@rly rly merged commit 8558a75 into dev Mar 24, 2026
20 of 28 checks passed
@rly rly deleted the herd-manager-mixin-improvements branch March 24, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants