feat(dynamictable): add ragged and doubly-ragged array column helpers#855
Draft
ehennestad wants to merge 7 commits into
Draft
feat(dynamictable): add ragged and doubly-ragged array column helpers#855ehennestad wants to merge 7 commits into
ehennestad wants to merge 7 commits into
Conversation
Add util.create_doubly_indexed_column plus the DynamicTable methods addRaggedArray and addDoublyRaggedArray for building ragged and doubly-ragged array columns (such as the Units waveforms column) without manually wiring the VectorData and VectorIndex objects. The methods live in matnwb.neurodata.DynamicTableBase so they are inherited by all table types and survive generateCore. Covered by new unit tests for both the helper and the methods. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #855 +/- ##
==========================================
+ Coverage 95.45% 95.50% +0.04%
==========================================
Files 228 229 +1
Lines 8080 8165 +85
==========================================
+ Hits 7713 7798 +85
Misses 367 367 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add a how-to guide for storing ragged and doubly-ragged array columns with addRaggedArray / addDoublyRaggedArray, covering single-electrode and multi-channel waveforms and the underlying util helpers. Code snippets live in a runnable example file, embedded via literalinclude and executed by a unit test so the documented code stays correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update UnitTimesIOTest to build a Units table with multi-electrode, doubly-ragged waveforms and compare MatNWB and PyNWB containers after a round trip. MatNWB uses addRaggedArray / addDoublyRaggedArray; PyNWB uses Units.add_unit. Both sides document the per-unit waveforms input order as (num_spikes, num_electrodes, num_samples). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
UnitTimes is a legacy neurodata type name that no longer exists; the table is the Units table. Rename the round-trip test class and file (and the matching PyNWBIOTest.py class) accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3 tasks
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.
Todo
Motivation
Background — NWB
DynamicTables store a variable number of items per row as ragged arrays (§2.5), and some columns — notably theUnitstable'swaveforms— use a doubly ragged array (§2.6) with two levels of indexing.Problem — MatNWB had no ergonomic way to build these. For a doubly-ragged column,
addRowsilently produces a non-conformant structure (a single index level, which schema-aware readers such as pynwb misinterpret), and doing it correctly meant hand-constructing aVectorDataplus twoVectorIndexobjects with the right cumulative offsets andObjectViewtargets. Even a single-level ragged column required the two-steputil.create_indexed_column+addColumndance.Solution — Add two
DynamicTablemethods,addRaggedArrayandaddDoublyRaggedArray(plus the underlyingutil.create_doubly_indexed_columnprimitive), that build and wire all the objects in a single call. The names mirror the NWB spec section titles.What changed
DynamicTable.addRaggedArray(name, data)— builds a ragged column (aVectorData+ itsVectorIndex) from a cell array and adds it, updatingcolnamesandid. An optionaltableargument produces aDynamicTableRegioncolumn instead.DynamicTable.addDoublyRaggedArray(name, data)— builds a doubly-ragged column (aVectorData+ twoVectorIndexlevels) and wiresname,name_index, andname_index_index.util.create_doubly_indexed_column— the low-level builder, mirroring the existingutil.create_indexed_column.Units,TimeIntervals,ElectrodesTable, …) and survivegenerateCore, since they live in the hand-writtenmatnwb.neurodata.DynamicTableBase.waveforms, and the underlying helpers. Its code snippets live in a runnable example file, embedded vialiteralincludeand executed by a unit test, so the documented code cannot drift from the API.Implementation notes
addDoublyRaggedArraywires the three objects directly rather than delegating toaddColumn, becauseaddColumn's height check only follows a single index level and would reject a doubly-ragged column.create_doubly_indexed_columnaccepts either a per-row[nSubGroups x nSamples]matrix (single-element shortcut, e.g. one electrode per unit) or a nested cell (data{i}{j} = [nElements x nSamples]), and validates that all elements share the same sample length.Examples
Doubly-ragged
Units.waveformsShared setup:
Before — manual construction of three objects:
After — one call:
Single-level ragged
Units.spike_timesBefore:
After:
How to test
New unit tests cover the helper, the methods, and the documentation examples:
Checklist
fix #XXwhereXXis the issue number?🤖 Generated with Claude Code