feat(herd): add a user-facing API for external resource references - #877
Open
ehennestad wants to merge 15 commits into
Open
feat(herd): add a user-facing API for external resource references#877ehennestad wants to merge 15 commits into
ehennestad wants to merge 15 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## fix-empty-compound-dataset-write #877 +/- ##
====================================================================
+ Coverage 95.33% 95.36% +0.03%
====================================================================
Files 234 237 +3
Lines 8352 8660 +308
====================================================================
+ Hits 7962 8259 +297
- Misses 390 401 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ehennestad
marked this pull request as draft
August 27, 2026 07:46
ehennestad
force-pushed
the
add-herd-user-api
branch
2 times, most recently
from
August 27, 2026 08:53
1be5f31 to
b0d04c6
Compare
ehennestad
force-pushed
the
add-herd-user-api
branch
2 times, most recently
from
August 27, 2026 16:14
6063815 to
0a71c4e
Compare
ehennestad
force-pushed
the
add-herd-user-api
branch
from
August 31, 2026 10:43
0c9093d to
33d24f6
Compare
ehennestad
force-pushed
the
add-herd-user-api
branch
from
August 31, 2026 14:25
33d24f6 to
a2456ab
Compare
ehennestad
commented
Aug 31, 2026
Add matnwb.neurodata.HERDBase, a non-generated base class for the
generated HERD type, and register it with the code generator. HERD
holds its associations in six tables that reference each other by
zero-based row index; the new methods hide that layout behind terms,
entities and objects:
herd.addRef(nwb, nwb.general_subject, Key="Mus musculus", ...
EntityId="NCBITaxon:10090", ...
EntityUri="http://purl.obolibrary.org/obo/NCBITaxon_10090")
Lookups are getEntity, getKey, getObjectEntities and getObjectType.
toTable flattens the six tables into one table of references.
Semantics follow HDMF: keys are scoped to an object rather than shared
across the file, an entity keeps the URI it was first stored with, and
one key may resolve to several entities. Unlike HDMF, adding a
reference that is already recorded changes nothing instead of
appending a duplicate object_keys row; both files read the same way.
Referencing an attribute is limited to properties that are themselves
neurodata types, such as a table column. Attributes holding plain
values need relative path support, which is not implemented yet.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add nwb.addRef(container, ...) and nwb.getExternalResources() as the primary entry points for HERD: addRef lazily creates and attaches the file's HERD on first use, matching PyNWB's get_external_resources semantics (a file has at most one HERD). HERD.addRef remains available directly for building a HERD before it is attached to a file. Give HERD a custom display showing the reference-table summary counts and the flattened table instead of the six internal Data properties, which are not informative on their own. This requires HERDBase to declare matlab.mixin.CustomDisplay directly: MATLAB treats a method defined in an unrelated class as ambiguous rather than as an override unless the class sits in the same CustomDisplay lineage. getFooter is left untouched so the existing missing-required-property warning from MetaClass still fires. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
getObjectTypeName used class(container), which returns the bare
'NwbFile' for the MatNWB-only wrapper class rather than the 'NWBFile'
schema name MetaClass.export actually writes to neurodata_type. This
mismatch was invisible for every other type (their class name already
equals the schema name) but meant an external reference added directly
to the file, via addRef(nwb, nwb, ...), recorded an object_type that
getObjectType("NWBFile") and any PyNWB/HDMF consumer filtering by
neurodata_type would never match, even though the same row's object_id
still resolved correctly.
Also drop a dead isempty/else branch in findKeyForObject: intersect
already returns empty when there is no match, so the branch reassigned
empty to empty.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Asking a file for its external resources attached a HERD whose six tables were unset, and every one of them is required by the schema. Exporting that file stopped with a required-property error listing all six, so nwb.getExternalResources() followed by nwbExport could not be run. PyNWB writes an empty HERD for the same sequence. The tables were only filled in by addRef, which left the HERD unwritable until a reference was added to it. getExternalResources now fills them in too, so the HERD it hands back can be written as it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generated class accepts a compound dataset as a struct array, but normalizing one called struct2table with AsArray false, which applies only to a scalar struct and is rejected for an array. Assigning a struct array to any of the six tables therefore failed with MATLAB:struct2table:NonScalar on the next lookup. Cover the shapes the lookups have to normalize, the two options getObjectType narrows by, and the arguments NwbFile.addRef declares itself rather than forwarding. Drop the note comparing the deduplication of repeated references against HDMF from the class docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Declaring container as types.untyped.MetaClass let an argument of the wrong type through. Argument validation converts a mismatched value by calling the class constructor, and that constructor accepts any input, so an unset property of a file, which is an empty double, was turned into an empty MetaClass rather than rejected. The reference then failed much later, against a class name that names an implementation detail rather than anything a user recognises. Validate the argument with matnwb.common.validation.mustBeNeurodataObject instead. A validator cannot improve the message while a class is the declared type, because the conversion is attempted first, so the type is dropped and the validator reports the problem itself: an empty value names the property that was never set, and any other value names the class it actually is. Describe container in the same terms in the docstrings, and give the See also blocks the layout the rest of the file uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A member type cannot be derived from an empty object reference column, so zeroRowTableToColumnStruct fell back to the variable length string type, committing a member type that contradicts the schema. Such a dataset can never be extended with real reference rows and does not read back as references, so the write is rejected instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The six HERD tables are required by the schema, so a HERD had to be routed through NwbFile.getExternalResources to become exportable. Initialize them from the generated constructor instead, using the same post-setup hook the generator already emits for DynamicTable, so every HERD can be written whether or not a reference is ever added to it. The required property check treated a row-less value as an omission unless it was a table, which warned for a compound value held as a struct array and let a table with no columns through to fail later against an HDF5 path. Check for compound structure rather than one MATLAB class. Reading a table back also collapsed a scalar struct whose first field was empty into an empty table, discarding data when the remaining fields held rows. struct2table already covers the empty case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The type name a reference is recorded under has to match the neurodata_type attribute export writes, so HERD derived it by stripping the namespace off the class name and special cased NwbFile, whose wrapper class name diverges from the type it is written as. Move that divergence onto NwbFile as a getTypeShortName override and take the name from that method, which is what other handwritten base classes already use. The identity of a row of the objects table was spelled out separately where a reference is added and where one is looked up, so a column added or reordered had to be edited in both. Build it in one place instead. NwbFile.addRef restated the name-value defaults that HERDBase.addRef already declares, overriding them with its own copy on every call. Declare the options without defaults so an option the caller omits is not forwarded and the rules stay single sourced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix error message, previously it assumed the input was handed as a property of another neurodata type, i.e nwbFile.general_subject
Verifying that a container belongs to a file went through NwbFile.searchFor, which walks every property of every object in the file and collects them into a path to object map before the membership test can begin. Adding a reference to each of many objects repeated that walk once per reference. Add matnwb.utility.containsObject, which answers the same question by walking the containment tree and stopping at the first match. It visits the same property, Set and Anon kinds as searchFor and descends only into the kinds that hold other objects, so it stays within the containment tree and cannot cycle. It does not visit a Set held by a type with the HasUnnamedGroups mixin, because that mixin exposes every member as a dynamic property of the parent, so the members have already been seen. That does not hold for an Anon, which the mixin does not support, so those are still visited. See issue #886. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A table read from a file arrives as a DataStub, and getTable loaded it on every call without keeping the result, so each lookup read the dataset from disk again. One addRef makes about ten lookups, and a HERD holds six tables, so annotating objects in a loop re-read the same datasets repeatedly. Keep the loaded dataset on the Data object instead. The row lookup used to take the number of leading columns to compare, which only ever served the entities table, and only because entity_id happens to be declared first. A reordering of that compound type would have matched a URI against an identifier, silently. Look the entity up by name at the two call sites that want it, the way getKey already does, and let the row matcher compare a whole row. It asserts that it was given one value per column, so a row that no longer fits its table fails instead of matching on the columns it happens to cover. findRow only forwarded to that matcher and is gone; its callers read the table and match directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The private helpers mixed two idioms: some were statics called through the fully qualified class name, others instance methods with an unused first argument. Settle on the instance idiom, which keeps call sites short. A table and a row of values were both passed around as `value`/`values`, side by side in the row matcher. Name them tableData and rowValues, and a key's text keyName, matching getKey. Document what was only discoverable by reading the implementation: that the public methods mirror the HDMF/PyNWB HERD API, that resolveTarget's relativePath is always empty until relative-path support exists, that getObjectEntities errors for an unannotated object rather than returning an empty table, and what RelativePath and Field narrow on. The query methods get the same argument sections addRef already had. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
HDMF's get_object_type matches rows whose relative_path and field equal the given values, which default to empty, and only returns every instance of the type when all_instances is set. getObjectType filtered on those columns only when an option was passed, so its default behaved like all_instances and would return more rows than the HDMF call once references with a relative path or field exist. Filter like HDMF, and add AllInstances for the every-instance form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ehennestad
force-pushed
the
add-herd-user-api
branch
from
September 1, 2026 11:40
10f0799 to
6af557c
Compare
ehennestad
marked this pull request as ready for review
September 1, 2026 11:40
The pynwb round-trip test carried the tag name from before the rename in #874, so the SKIP_PYNWB_TESTS filter, which selects on RequiresPython, did not catch it and it ran on the R2021b and R2022a CI runners, where pynwb is never installed. It was also the only test file calling py.* functions directly; the other Python tests shell out. On R2021a, whose Python is unsupported by MATLAB, those references fail suite construction for the whole file, silently dropping every HERD test on that release. Move the pynwb-side reads into tests.util.readHerdCountsWithPynwb so the test file holds no py.* references and its other tests run everywhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Problem — A value in an NWB file is often a term that means something specific: a species name, a brain region, a unit. HERD (HDMF External Resources Data Structure) is the schema type that records what those terms refer to in an external resource such as an ontology, so the values are standardized rather than free text. MatNWB generated the
HERDclass but gave no way to use it: adding one annotation meant assembling six compound tables by hand and wiring them together with zero-based row indices, and getting an annotation back out meant reversing that by hand.Solution — Add the operations HERD is actually used for. One call records that an object refers to an external entity, and a small set of lookups reads the annotations back as MATLAB tables.
What changed
nwb.addRef(container, ...)records that an object in the file refers to an external entity.nwb.getExternalResources()returns the file's HERD, creating and attaching one on first use. A file has at most one HERD, so an existing one is reused, including afternwbRead.HERD.toTable()flattens the six tables into one table of references, with the internal row indices resolved into the values they point at.HERD.getEntity,getKey,getObjectEntitiesandgetObjectTypelook annotations back up.Dataproperties.Deferred
Two pieces of the HDMF HERD API are deliberately left out of this PR:
References to plain-value attributes (
relative_path). HDMF'sadd_refcan target an attribute that is not a neurodata type — e.g. annotatingSubject.speciesitself rather than the Subject — by recording a schema-relative path to it (TimeSeries.unitbecomesdata/unit). That path is resolved at add time, purely from the schema, through HDMF's runtime ObjectMapper, which maps every attribute of a live object back to its spec node. MatNWB has no such runtime mapping: the schema is consumed at code-generation time and nested structure is flattened into underscore-joined properties (data/unit→data_unit), which discards the property-to-spec-path relation and makes it ambiguous to reverse from the name alone. Supporting this needs a spec-walker over the cached namespace specs — a self-contained follow-up, not an architectural blocker. Until then, referencing an attribute that is itself a neurodata type (such as a column of aDynamicTable) works, other attributes raiseNWB:HERD:UnsupportedAttributerather than writing a wrong path, and MatNWB always writesrelative_pathempty. Files written by HDMF that already hold relative paths still read fine:toTablecarries the column andgetObjectTypefilters on it.The sidecar zip form (
to_zip/from_zip). HDMF can save a HERD as a zip of TSV files so one set of annotations can be shared across NWB files. Not implemented here; a HERD lives only inside its file.Implementation notes
The handwritten behaviour lives in
matnwb.neurodata.HERDBase, an abstract base class attached to the generatedHERDclass through thecustomBaseClassesmap infile.fillClass, following the existingDynamicTableBaseandAlignedDynamicTableBasepattern.Row indices are stored zero-based on disk to match HDMF and PyNWB, and are converted at the API boundary so no public method takes or returns one.
Semantics follow HDMF: keys are scoped to an object rather than shared across the file, so the same term on two objects is stored as two key rows; an entity keeps the URI it was first stored with and a differing URI warns; and one key may resolve to several entities. Adding a reference that is already recorded changes nothing, where HDMF appends a second identical
object_keysrow; both files read the same way.HERDBasedeclaresmatlab.mixin.CustomDisplaydirectly. MATLAB treats a display hook defined in a class outside that lineage as an ambiguous definition rather than an override, so the mixin has to sit in it.getFooteris left toMetaClassso its missing-property warning still fires.Examples
Recording and reading back an annotation
Before — neither entry point exists, so the six tables have to be built by hand:
and the result displays as its raw properties:
After
Looking up what is annotated on one object
Before — no such method; the
objects,object_keys,entity_keysandentitiestables have to be joined by hand on their zero-based indices.After
How to test
Unit tests:
Todo
Checklist
fix #XXwhereXXis the issue number?🤖 Generated with Claude Code