Skip to content

Add DynamicTable non-generated base class#838

Open
ehennestad wants to merge 10 commits into
codex/aligned-dynamic-table-add-categoryfrom
add-dynamictable-base-class
Open

Add DynamicTable non-generated base class#838
ehennestad wants to merge 10 commits into
codex/aligned-dynamic-table-add-categoryfrom
add-dynamictable-base-class

Conversation

@ehennestad

@ehennestad ehennestad commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Depends on #837 — merge that first

Motivation

DynamicTable methods (addRow, addColumn, getRow, toTable, clear) were previously injected into generated class files as thin delegators to utility functions in +types/+util/+dynamictable/. Because the generator writes methods as multiline strings, adding docstrings or arguments blocks there is impractical — so none existed.

The consequence was poor discoverability: no in-editor help text, no autocomplete hints for name-value arguments, and no input validation at the method boundary. Users had to trace through to the underlying utility functions to understand how to call the methods.

This PR introduces matnwb.neurodata.DynamicTableBase, a handwritten abstract base class that DynamicTable and its subclasses inherit from. The delegation to utility functions is unchanged — the only additions are docstrings and arguments blocks on each method, making them self-documenting and IDE-friendly.

The toTable method also renames the positional argument from index to keepRegionsIndexed for clarity.

How to test the behavior?

t = types.hdmf_common.DynamicTable( ...
    'description', 'test table', ...
    'colnames', {'x'}, ...
    'x', types.hdmf_common.VectorData('description', 'x col', 'data', [1; 2; 3]), ...
    'id', types.hdmf_common.ElementIdentifiers('data', int64([0; 1; 2])) ...
);

t.addRow('x', 4);
t.addColumn('y', types.hdmf_common.VectorData('description', 'y col', 'data', [10; 20; 30; 40]));
disp(t.getRow(1:2));
disp(t.toTable());
t.clear();

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

@ehennestad ehennestad changed the title Add DynamicTableBase non-generated base class Add DynamicTable non-generated base class Jun 26, 2026
@ehennestad ehennestad mentioned this pull request Jun 26, 2026
5 tasks
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (codex/aligned-dynamic-table-add-category@19a73bd). Learn more about missing BASE report.

Files with missing lines Patch % Lines
+matnwb/+neurodata/DynamicTableBase.m 94.28% 2 Missing ⚠️
Additional details and impacted files
@@                             Coverage Diff                             @@
##             codex/aligned-dynamic-table-add-category     #838   +/-   ##
===========================================================================
  Coverage                                            ?   95.38%           
===========================================================================
  Files                                               ?      220           
  Lines                                               ?     7894           
  Branches                                            ?        0           
===========================================================================
  Hits                                                ?     7530           
  Misses                                              ?      364           
  Partials                                            ?        0           

☔ View full report in Codecov by Harness.
📢 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.

Remove test validating a deprecating error that is removed in this PR
Remove test verifying a deprecating error that is removed in this PR
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.

1 participant