From d436ce3f930d1709afa681d47ca9c72bb36a6d89 Mon Sep 17 00:00:00 2001 From: Eivind Hennestad Date: Wed, 1 Jul 2026 15:53:10 +0200 Subject: [PATCH 1/9] Add DynamicTable non-generated base class (#838) * Add DynamicTableBase class for handwritten DynamicTable behavior Introduce matnwb.neurodata.DynamicTableBase, an abstract handle base class that owns the handwritten DynamicTable behavior the generated schema class cannot express: row and column mutation, row retrieval, table conversion, clearing, and consistency validation. Co-Authored-By: Claude Opus 4.8 * Generate DynamicTable classes with a custom base class fillClass attaches matnwb.neurodata.DynamicTableBase as a superclass for DynamicTable via a customBaseClasses map, rather than injecting table methods inline. The constructor and custom-constraint hooks now route the consistency check through obj.ensureDynamicTableConsistency(). Remove the now-unused fillDynamicTableMethods. Co-Authored-By: Claude Opus 4.8 * Regenerate types against DynamicTableBase DynamicTable now inherits matnwb.neurodata.DynamicTableBase and drops its inline table methods; DynamicTable descendants call obj.ensureDynamicTableConsistency() from their constructors and custom-constraint checks. Co-Authored-By: Claude Opus 4.8 * Remove obsolete dynamictable add* wrappers addColumn, addRow, and addTableColumn are superseded by DynamicTableBase methods that call the addVararg* helpers directly. Document addVarargRow. Co-Authored-By: Claude Opus 4.8 * Update DynamicTable tests for new addRow/addColumn signatures Remove assertions that passing a MATLAB table to addRow/addColumn throws NWB:DynamicTable; the new repeating-argument signatures reject that input via argument validation instead. Co-Authored-By: Claude Opus 4.8 * Route clear via the DynamicTable's clear method in dynamicTableTest --------- Co-authored-by: Claude Opus 4.8 --- +file/fillClass.m | 12 +- +file/fillConstructor.m | 2 +- +file/fillCustomConstraint.m | 2 +- +file/fillDynamicTableMethods.m | 23 -- +matnwb/+neurodata/DynamicTableBase.m | 222 ++++++++++++++++++++ +tests/+sanity/GenerationTest.m | 7 - +tests/+system/DynamicTableTest.m | 8 - +tests/+unit/dynamicTableTest.m | 2 +- +types/+core/ElectrodesTable.m | 2 +- +types/+core/ExperimentalConditionsTable.m | 2 +- +types/+core/FrequencyBandsTable.m | 2 +- +types/+core/IntracellularElectrodesTable.m | 2 +- +types/+core/IntracellularRecordingsTable.m | 2 +- +types/+core/IntracellularResponsesTable.m | 2 +- +types/+core/IntracellularStimuliTable.m | 2 +- +types/+core/PlaneSegmentation.m | 2 +- +types/+core/RepetitionsTable.m | 2 +- +types/+core/SequentialRecordingsTable.m | 2 +- +types/+core/SimultaneousRecordingsTable.m | 2 +- +types/+core/SweepTable.m | 2 +- +types/+core/TimeIntervals.m | 2 +- +types/+core/Units.m | 2 +- +types/+hdmf_common/AlignedDynamicTable.m | 2 +- +types/+hdmf_common/DynamicTable.m | 26 +-- +types/+util/+dynamictable/addColumn.m | 38 ---- +types/+util/+dynamictable/addRow.m | 49 ----- +types/+util/+dynamictable/addTableColumn.m | 4 - +types/+util/+dynamictable/addVarargRow.m | 20 ++ 28 files changed, 271 insertions(+), 174 deletions(-) delete mode 100644 +file/fillDynamicTableMethods.m create mode 100644 +matnwb/+neurodata/DynamicTableBase.m delete mode 100644 +types/+util/+dynamictable/addColumn.m delete mode 100644 +types/+util/+dynamictable/addRow.m delete mode 100644 +types/+util/+dynamictable/addTableColumn.m diff --git a/+file/fillClass.m b/+file/fillClass.m index 2dd6c3296..19320c449 100644 --- a/+file/fillClass.m +++ b/+file/fillClass.m @@ -2,6 +2,10 @@ %name is the name of the scheme %namespace is the namespace context for this class + customBaseClasses = struct( ... + 'DynamicTable', 'matnwb.neurodata.DynamicTableBase' ... + ); + %% PROCESSING class = processed(1); @@ -96,6 +100,10 @@ superclassNames{end+1} = 'matnwb.mixin.HasUnnamedGroups'; end + if isfield(customBaseClasses, name) + superclassNames{end+1} = customBaseClasses.(name); + end + %% return classfile string classDefinitionHeader = [... 'classdef ' name ' < ' strjoin(superclassNames, ' & ') newline... %header, dependencies @@ -172,10 +180,6 @@ methodBody = strjoin({methodBody, '%% CUSTOM CONSTRAINTS', customConstraintStr}, newline); end - if strcmp(name, 'DynamicTable') - methodBody = strjoin({methodBody, '%% TABLE METHODS', file.fillDynamicTableMethods()}, newline); - end - fullMethodBody = strjoin({'methods' ... file.addSpaces(methodBody, 4) 'end'}, newline); readPolicyMethodBlock = fillReadPolicy(class, classprops); diff --git a/+file/fillConstructor.m b/+file/fillConstructor.m index b523c9a3c..aa1e1dce1 100644 --- a/+file/fillConstructor.m +++ b/+file/fillConstructor.m @@ -29,7 +29,7 @@ % Add custom validation for DynamicTable and its descendant classes if file.isDynamicTableDescendant(name, namespace) - constructorElements{end+1} = ' types.util.dynamictable.checkConfig(obj);'; + constructorElements{end+1} = ' obj.ensureDynamicTableConsistency();'; end constructorElements{end+1} = 'end'; diff --git a/+file/fillCustomConstraint.m b/+file/fillCustomConstraint.m index 992a2ffc8..8072a9a6c 100644 --- a/+file/fillCustomConstraint.m +++ b/+file/fillCustomConstraint.m @@ -38,7 +38,7 @@ customConstraintStr = sprintf( [... 'function checkCustomConstraint(obj)\n', ... ' checkCustomConstraint@types.untyped.MetaClass(obj)\n', ... - ' types.util.dynamictable.checkConfig(obj)\n', ... + ' obj.ensureDynamicTableConsistency()\n', ... 'end'] ); otherwise diff --git a/+file/fillDynamicTableMethods.m b/+file/fillDynamicTableMethods.m deleted file mode 100644 index 8af595b81..000000000 --- a/+file/fillDynamicTableMethods.m +++ /dev/null @@ -1,23 +0,0 @@ -function methodStr = fillDynamicTableMethods() -methodStr = strjoin({... - 'function addRow(obj, varargin)',... - ' types.util.dynamictable.addRow(obj, varargin{:});',... - 'end',... - '',... - 'function addColumn(obj, varargin)',... - ' types.util.dynamictable.addColumn(obj, varargin{:});',... - 'end',... - '',... - 'function row = getRow(obj, id, varargin)',... - ' row = types.util.dynamictable.getRow(obj, id, varargin{:});',... - 'end',... - '',... - 'function table = toTable(obj, varargin)',... - ' table = types.util.dynamictable.nwbToTable(obj, varargin{:});',... - 'end',... - '',... - 'function clear(obj)',... - ' types.util.dynamictable.clear(obj);',... - 'end'}, newline); -end - diff --git a/+matnwb/+neurodata/DynamicTableBase.m b/+matnwb/+neurodata/DynamicTableBase.m new file mode 100644 index 000000000..d638191ec --- /dev/null +++ b/+matnwb/+neurodata/DynamicTableBase.m @@ -0,0 +1,222 @@ +classdef (Abstract) DynamicTableBase < handle +% DynamicTableBase - Non-generated base class for DynamicTable behavior. +% +% This class owns handwritten DynamicTable behavior that the generated +% schema class cannot express: row and column mutation helpers, row +% retrieval, table conversion, table clearing, and DynamicTable consistency +% validation. + + properties (Abstract) + id + colnames + end + + methods + function addRow(obj, columnName, columnValue, options) + % addRow - Add a single row to the DynamicTable. + % + % Syntax: + % dynamicTable.addRow(columnName, columnValue, ..., columnNameN, columnValueN) + % append a single row to the DynamicTable. + % + % dynamicTable.addRow(__, Name, Value) add a row, providing an + % optional value for the 'id' using the optional 'id' name-value + % argument + % + % Input Arguments (Repeating): + % - columnName (string) - + % Name of a column in the table. + % + % - columnValue (any) - + % Corresponding value for the preceding columnName. + % + % Name-Value Arguments: + % - id (int) - + % A custom value for the id of the row being added. + + arguments + obj (1,1) matnwb.neurodata.DynamicTableBase {matnwb.common.validation.mustBeDynamicTable} + end + arguments (Repeating) + columnName (1,1) string + columnValue + end + arguments + options.id + end + + assert(~isempty(columnName), ... + 'NWB:DynamicTable:AddRow:NoData', 'Not enough arguments') + + obj.assertIsEditable('NWB:DynamicTable:AddRow:Uneditable') + + assert(~isempty(obj.colnames), ... + 'NWB:DynamicTable:AddRow:NoColumns',... + ['The `colnames` property of the DynamicTable needs to be ', ... + 'populated with a cell array of column names before being ', ... + 'able to add row data.']) + + obj.ensureDynamicTableConsistency() + + columnValuePairs = [columnName; columnValue]; + optionalArgs = namedargs2cell(options); + + types.util.dynamictable.addVarargRow(obj, columnValuePairs{:}, optionalArgs{:}); + end + + function addColumn(obj, columnName, columnVector) + % addColumn - Add one or more columns to the DynamicTable. + % + % Given a dynamic table and a set of keyword arguments for one or + % more columns, add one or more columns to the dynamic table by + % providing name-value pairs where the name is a column name and + % the value is a column vector + % + % Syntax: + % dynamicTable.addColumn(columnName, columnVector) + % add a single column to the DynamicTable. + % + % dynamicTable.addColumn(columnName, columnVector, ..., columnNameN, columnVectorN) + % add many new columns to the DynamicTable + % + % Input Arguments (Repeating): + % - columnName (string) - + % Name of the new column in the table. + % + % - columnVector (VectorData | VectorIndex) - + % Corresponding VectorData or VectorIndex for the new column + % + % Note: + % The height of the columns to be appended must match the height of + % the existing columns + + arguments + obj (1,1) {matnwb.common.validation.mustBeDynamicTable} + end + + arguments (Repeating) + columnName (1,1) string + columnVector + end + + assert(~isempty(columnName), ... + 'NWB:DynamicTable:AddColumn:NoData', 'Not enough arguments') + + if isempty(obj.id) + types.util.dynamictable.internal.initDynamicTableId(obj); + end + + obj.assertIsEditable('NWB:DynamicTable:AddColumn:Uneditable') + + columnVectorPairs = [columnName; columnVector]; + types.util.dynamictable.addVarargColumn(obj, columnVectorPairs{:}); + end + + function row = getRow(obj, rowIndices, options) + % getRow - Return one or more DynamicTable rows. + % + % Syntax: + % dynamicTable.getRow(rowIndices) return one or more rows of the + % table given a scalar row index or a list of row indices. + % + % dynamicTable.getRow(rowIndices, Name, Value) get rows providing + % optional name-value pairs for customization (see Input Arguments). + % + % Input Arguments: + % - rowIndices (double) - + % A scalar index or a vector of row indices for rows to extract. + % Must be positive integers, respecting the row count of the table. + % + % - options (name-value pairs) - + % Optional name-value pairs. Available options: + % + % - columns (string) - + % A list of names of columns to retrieve. Allows for only + % grabbing certain columns instead of returning all columns. + % + % - useId (logical) - + % If true, rowIndices refer to the table's id column instead + % of the MATLAB-based row indices. + % + % Output Arguments: + % - row (table) - + % A table of specified rows, with columns ordered according to + % the DynamicTable's colnames property, or the values given for + % the "columns" option if provided. + + arguments + obj (1,1) {matnwb.common.validation.mustBeDynamicTable} + rowIndices (1,:) double {mustBeInteger} + options.columns (1,:) + options.useId (1,1) logical + end + + nvPairs = namedargs2cell(options); + row = types.util.dynamictable.getRow(obj, rowIndices, nvPairs{:}); + end + + function table = toTable(obj, keepRegionsIndexed) + % toTable - Convert the DynamicTable to a MATLAB table. + % + % Syntax: + % dynamicTable.toTable() converts the DynamicTable object to a + % MATLAB table. DynamicTableRegion columns are kept as index + % references by default. + % + % dynamicTable.toTable(keepRegionsIndexed) controls how + % DynamicTableRegion columns are represented (see Input Arguments). + % + % Input Arguments: + % - keepRegionsIndexed (logical) - + % When true (default), each DynamicTableRegion column is preserved + % as row indices into the referenced table. When false, each + % DynamicTableRegion column is expanded into a nested subtable of + % the referenced rows. + + arguments + obj (1,1) {matnwb.common.validation.mustBeDynamicTable} + keepRegionsIndexed (1,1) logical = true + end + + table = types.util.dynamictable.nwbToTable(obj, keepRegionsIndexed); + end + + function clear(obj) + % clear - Remove all row and column data from the DynamicTable. + % + % Resets the table to an empty state: all VectorData columns, + % VectorIndex columns, and row ids are cleared. The colnames + % property is preserved. + + types.util.dynamictable.clear(obj); + end + end + + methods (Hidden) + function ensureDynamicTableConsistency(obj) + % ensureDynamicTableConsistency - Ensure DynamicTable column consistency. + % + % This method validates column registration, row-height consistency, + % compound column shape, VectorIndex chains, and id height. It may + % also initialize missing ids when the table height can be inferred + % from materialized columns. + + types.util.dynamictable.checkConfig(obj); + end + end + + methods (Access = private) + function assertIsEditable(obj, errorID) + arguments + obj (1,1) matnwb.neurodata.DynamicTableBase + errorID (1,1) string = "NWB:DynamicTable:Uneditable" + end + + isEditable = ~isa(obj.id.data, 'types.untyped.DataStub'); + + assert(isEditable, errorID, ... + ['Cannot write to on-file Dynamic Tables without enabling data pipes. '... + 'If this was produced with pynwb, please enable chunking for this table.']); + end + end +end diff --git a/+tests/+sanity/GenerationTest.m b/+tests/+sanity/GenerationTest.m index 7ff45f88c..2a884e8d9 100644 --- a/+tests/+sanity/GenerationTest.m +++ b/+tests/+sanity/GenerationTest.m @@ -65,13 +65,6 @@ function dynamicTableMethodsTest(testCase) 'stringdata', {'TRUE'},... 'id', id(i)); end - t = table(id(6:10), (6:10)', (7:11)', ... - rand(5,1), repmat({'TRUE'}, 5, 1), ... - 'VariableNames', {'id', 'start_time', 'stop_time', 'randomvalues', 'stringdata'}); - % verify error is thrown when addRow input is MATLAB table - testCase.verifyError(@() TimeIntervals.addRow(t), ... - "NWB:DynamicTable" ... - ); retrievalIndex = round(1 + 4 .* rand(10, 1)); indexedRow = TimeIntervals.getRow(retrievalIndex); diff --git a/+tests/+system/DynamicTableTest.m b/+tests/+system/DynamicTableTest.m index 551b1fe95..432ef28d9 100644 --- a/+tests/+system/DynamicTableTest.m +++ b/+tests/+system/DynamicTableTest.m @@ -117,14 +117,6 @@ function appendContainer(testCase, file) 'data', (20:-1:1) .' ... ) ... ); - % verify error is thrown when addRow input is MATLAB table - t = table( ... - (1:2:40)', (1:4:80)' , ... - 'VariableNames', {'newcolumn2', 'newcolumn3'} ... - ); - testCase.verifyError(@() file.intervals_trials.addColumn(t), ... - "NWB:DynamicTable" ... - ); end function appendRaggedContainer(~, file) diff --git a/+tests/+unit/dynamicTableTest.m b/+tests/+unit/dynamicTableTest.m index ca3abe1a8..14a64dff3 100644 --- a/+tests/+unit/dynamicTableTest.m +++ b/+tests/+unit/dynamicTableTest.m @@ -145,7 +145,7 @@ function toTableNdArrayTest(testCase) function testClearDynamicTable(testCase) dtr_table = testCase.createDynamicTableWithTableRegionReferences(); - types.util.dynamictable.clear(dtr_table) + dtr_table.clear() % testCase.verifyEmpty(dtr_table.vectordata) %todo when PR merged testCase.verifyEqual(size(dtr_table.vectordata), uint64([0,1])) diff --git a/+types/+core/ElectrodesTable.m b/+types/+core/ElectrodesTable.m index 023ad118d..6e6f14e42 100644 --- a/+types/+core/ElectrodesTable.m +++ b/+types/+core/ElectrodesTable.m @@ -107,7 +107,7 @@ if strcmp(class(obj), 'types.core.ElectrodesTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/ExperimentalConditionsTable.m b/+types/+core/ExperimentalConditionsTable.m index ba671e1fa..2c15a342f 100644 --- a/+types/+core/ExperimentalConditionsTable.m +++ b/+types/+core/ExperimentalConditionsTable.m @@ -54,7 +54,7 @@ if strcmp(class(obj), 'types.core.ExperimentalConditionsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/FrequencyBandsTable.m b/+types/+core/FrequencyBandsTable.m index 8a97194cb..e6d32eb3b 100644 --- a/+types/+core/FrequencyBandsTable.m +++ b/+types/+core/FrequencyBandsTable.m @@ -67,7 +67,7 @@ if strcmp(class(obj), 'types.core.FrequencyBandsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/IntracellularElectrodesTable.m b/+types/+core/IntracellularElectrodesTable.m index 3ceda52da..b8ee2aa71 100644 --- a/+types/+core/IntracellularElectrodesTable.m +++ b/+types/+core/IntracellularElectrodesTable.m @@ -48,7 +48,7 @@ if strcmp(class(obj), 'types.core.IntracellularElectrodesTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/IntracellularRecordingsTable.m b/+types/+core/IntracellularRecordingsTable.m index 02f5733fb..3a4a6ed15 100644 --- a/+types/+core/IntracellularRecordingsTable.m +++ b/+types/+core/IntracellularRecordingsTable.m @@ -62,7 +62,7 @@ if strcmp(class(obj), 'types.core.IntracellularRecordingsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/IntracellularResponsesTable.m b/+types/+core/IntracellularResponsesTable.m index 2f356d343..f546c8cb4 100644 --- a/+types/+core/IntracellularResponsesTable.m +++ b/+types/+core/IntracellularResponsesTable.m @@ -48,7 +48,7 @@ if strcmp(class(obj), 'types.core.IntracellularResponsesTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/IntracellularStimuliTable.m b/+types/+core/IntracellularStimuliTable.m index 15a65f38f..11ce3107e 100644 --- a/+types/+core/IntracellularStimuliTable.m +++ b/+types/+core/IntracellularStimuliTable.m @@ -56,7 +56,7 @@ if strcmp(class(obj), 'types.core.IntracellularStimuliTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/PlaneSegmentation.m b/+types/+core/PlaneSegmentation.m index b1171e3cc..fc976e878 100644 --- a/+types/+core/PlaneSegmentation.m +++ b/+types/+core/PlaneSegmentation.m @@ -82,7 +82,7 @@ if strcmp(class(obj), 'types.core.PlaneSegmentation') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/RepetitionsTable.m b/+types/+core/RepetitionsTable.m index 8b07f4b2b..4cf264541 100644 --- a/+types/+core/RepetitionsTable.m +++ b/+types/+core/RepetitionsTable.m @@ -54,7 +54,7 @@ if strcmp(class(obj), 'types.core.RepetitionsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/SequentialRecordingsTable.m b/+types/+core/SequentialRecordingsTable.m index 675de3bfd..5c1a65bcb 100644 --- a/+types/+core/SequentialRecordingsTable.m +++ b/+types/+core/SequentialRecordingsTable.m @@ -59,7 +59,7 @@ if strcmp(class(obj), 'types.core.SequentialRecordingsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/SimultaneousRecordingsTable.m b/+types/+core/SimultaneousRecordingsTable.m index a2917d3b5..ff9e9d45a 100644 --- a/+types/+core/SimultaneousRecordingsTable.m +++ b/+types/+core/SimultaneousRecordingsTable.m @@ -54,7 +54,7 @@ if strcmp(class(obj), 'types.core.SimultaneousRecordingsTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/SweepTable.m b/+types/+core/SweepTable.m index 22028ec4a..be52bf527 100644 --- a/+types/+core/SweepTable.m +++ b/+types/+core/SweepTable.m @@ -59,7 +59,7 @@ if strcmp(class(obj), 'types.core.SweepTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/TimeIntervals.m b/+types/+core/TimeIntervals.m index d5434d496..2ca975b4d 100644 --- a/+types/+core/TimeIntervals.m +++ b/+types/+core/TimeIntervals.m @@ -77,7 +77,7 @@ if strcmp(class(obj), 'types.core.TimeIntervals') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+core/Units.m b/+types/+core/Units.m index 607e02b26..e41ccf718 100644 --- a/+types/+core/Units.m +++ b/+types/+core/Units.m @@ -137,7 +137,7 @@ if strcmp(class(obj), 'types.core.Units') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+hdmf_common/AlignedDynamicTable.m b/+types/+hdmf_common/AlignedDynamicTable.m index 7004e18b3..635992014 100644 --- a/+types/+hdmf_common/AlignedDynamicTable.m +++ b/+types/+hdmf_common/AlignedDynamicTable.m @@ -60,7 +60,7 @@ cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); obj.setupHasUnnamedGroupsMixin(); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS diff --git a/+types/+hdmf_common/DynamicTable.m b/+types/+hdmf_common/DynamicTable.m index 0bf219716..6c1074793 100644 --- a/+types/+hdmf_common/DynamicTable.m +++ b/+types/+hdmf_common/DynamicTable.m @@ -1,4 +1,4 @@ -classdef DynamicTable < types.hdmf_common.Container & types.untyped.GroupClass +classdef DynamicTable < types.hdmf_common.Container & types.untyped.GroupClass & matnwb.neurodata.DynamicTableBase % DYNAMICTABLE - A group containing multiple datasets that are aligned on the first dimension (Currently, this requirement if left up to APIs to check and enforce). These datasets represent different columns in the table. Apart from a column that contains unique identifiers for each row, there are no other required datasets. Users are free to add any number of custom VectorData objects (columns) here. DynamicTable also supports ragged array columns, where each element can be of a different size. To add a ragged array column, use a VectorIndex type to index the corresponding VectorData type. See documentation for VectorData and VectorIndex for more details. Unlike a compound data type, which is analogous to storing an array-of-structs, a DynamicTable can be thought of as a struct-of-arrays. This provides an alternative structure to choose from when optimizing storage for anticipated access patterns. Additionally, this type provides a way of creating a table without having to define a compound type up front. Although this convenience may be attractive, users should think carefully about how data will be accessed. DynamicTable is more appropriate for column-centric access, whereas a dataset with a compound type would be more appropriate for row-centric access. Finally, data size should also be taken into account. For small tables, performance loss may be an acceptable trade-off for the flexibility of a DynamicTable. % % Required Properties: @@ -58,7 +58,7 @@ if strcmp(class(obj), 'types.hdmf_common.DynamicTable') %#ok cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); - types.util.dynamictable.checkConfig(obj); + obj.ensureDynamicTableConsistency(); end end %% SETTERS @@ -114,27 +114,7 @@ %% CUSTOM CONSTRAINTS function checkCustomConstraint(obj) checkCustomConstraint@types.untyped.MetaClass(obj) - types.util.dynamictable.checkConfig(obj) - end - %% TABLE METHODS - function addRow(obj, varargin) - types.util.dynamictable.addRow(obj, varargin{:}); - end - - function addColumn(obj, varargin) - types.util.dynamictable.addColumn(obj, varargin{:}); - end - - function row = getRow(obj, id, varargin) - row = types.util.dynamictable.getRow(obj, id, varargin{:}); - end - - function table = toTable(obj, varargin) - table = types.util.dynamictable.nwbToTable(obj, varargin{:}); - end - - function clear(obj) - types.util.dynamictable.clear(obj); + obj.ensureDynamicTableConsistency() end end diff --git a/+types/+util/+dynamictable/addColumn.m b/+types/+util/+dynamictable/addColumn.m deleted file mode 100644 index 3d2d55ce0..000000000 --- a/+types/+util/+dynamictable/addColumn.m +++ /dev/null @@ -1,38 +0,0 @@ -function addColumn(DynamicTable, varargin) -% ADDCOLUMN Given a dynamic table and a set of keyword arguments for one or -% more columns, add one or more columns to the dynamic table by providing -% either keywords or a MATLAB table -% -% ADDCOLUMN(DT,TABLE) append the columns of the MATLAB Table TABLE to the -% DynamicTable -% -% ADDCOLUMN(DT,col_name1,col_vector1,...,col_namen,col_vectorn) -% append specified column names and VectorData to table -% -% This function asserts the following: -% 1) DynamicTable is a valid dynamic table and has the correct -% properties. -% 2) The height of the columns to be appended matches the height of the -% existing columns - -validateattributes(DynamicTable,... - {'types.core.DynamicTable', 'types.hdmf_common.DynamicTable'},... - {'scalar'}); - -assert(nargin > 1, 'NWB:DynamicTable:AddColumn:NoData', 'Not enough arguments'); - - -if isempty(DynamicTable.id) - types.util.dynamictable.internal.initDynamicTableId(DynamicTable); -end - -assert(~isa(DynamicTable.id.data, 'types.untyped.DataStub'),... - 'NWB:DynamicTable:AddColumn:Uneditable',... - ['Cannot write to on-file Dynamic Tables without enabling data pipes. '... - 'If this was produced with pynwb, please enable chunking for this table.']); - -if istable(varargin{1}) - types.util.dynamictable.addTableColumn(DynamicTable, varargin{:}); -else - types.util.dynamictable.addVarargColumn(DynamicTable, varargin{:}); -end diff --git a/+types/+util/+dynamictable/addRow.m b/+types/+util/+dynamictable/addRow.m deleted file mode 100644 index 328223c15..000000000 --- a/+types/+util/+dynamictable/addRow.m +++ /dev/null @@ -1,49 +0,0 @@ -function addRow(DynamicTable, varargin) -% ADDROW Given a dynamic table and a set of keyword arguments for the row, -% add a single row to the dynamic table if using keywords, or multiple rows -% if using a table. -% -% ADDROW(DT,table) append the MATLAB table to the DynamicTable -% -% ADDROW(DT,col1,val1,col2,val2,...,coln,valn) append a single row -% to the DynamicTable -% -% ADDROW(DT,___,Name,Value) optional 'id' -% -% This function asserts the following: -% 1) DynamicTable is a valid dynamic table and has the correct -% properties. -% 2) The given keyword argument names match one of those ALREADY specified -% by the DynamicTable (that is, colnames MUST be filled out). -% 3) If the dynamic table is non-empty, the types of the column value MUST -% match the keyword value. -% 4) All horizontal data must match the width of the rest of the rows. -% Variable length strings should use cell arrays each row. -% 5) The type of the data cannot be a cell array of numeric values if using -% keyword arguments. For table appending mode, this is how ragged arrays -% are represented. - -validateattributes(DynamicTable,... - {'types.core.DynamicTable', 'types.hdmf_common.DynamicTable'},... - {'scalar'}); -assert(~isempty(DynamicTable.colnames),... - 'NWB:DynamicTable:AddRow:NoColumns',... - ['The `colnames` property of the Dynamic Table needs to be populated with a cell array '... - 'of column names before being able to add row data.']); -assert(nargin > 1, 'NWB:DynamicTable:AddRow:NoData', 'Not enough arguments'); - -types.util.dynamictable.checkConfig(DynamicTable); - -assert(~isa(DynamicTable.id.data, 'types.untyped.DataStub'),... - 'NWB:DynamicTable:AddRow:Uneditable',... - ['Cannot write to on-file Dynamic Tables without enabling data pipes. '... - 'If this was produced with pynwb, please enable chunking for this table.']); - -if istable(varargin{1}) - error('NWB:DynamicTable', ... - ['Using MATLAB tables as input to the addRow DynamicTable method has '... - 'been deprecated. Please, use key-value pairs instead']); -else - types.util.dynamictable.addVarargRow(DynamicTable, varargin{:}); -end -end \ No newline at end of file diff --git a/+types/+util/+dynamictable/addTableColumn.m b/+types/+util/+dynamictable/addTableColumn.m deleted file mode 100644 index dfdba5349..000000000 --- a/+types/+util/+dynamictable/addTableColumn.m +++ /dev/null @@ -1,4 +0,0 @@ -function addTableColumn(DynamicTable, subTable) -error('NWB:DynamicTable', ... - ['Using MATLAB tables as input to the addColumn DynamicTable method has '... - 'been deprecated. Please, use key-value pairs instead']) \ No newline at end of file diff --git a/+types/+util/+dynamictable/addVarargRow.m b/+types/+util/+dynamictable/addVarargRow.m index 3a37c322b..9f8241bc7 100644 --- a/+types/+util/+dynamictable/addVarargRow.m +++ b/+types/+util/+dynamictable/addVarargRow.m @@ -1,4 +1,24 @@ function addVarargRow(DynamicTable, varargin) +% ADDVARARGROW Given a dynamic table and a set of keyword arguments for the row, +% add a single row to the dynamic table if using keywords, or multiple rows +% if using a table. +% +% ADDVARARGROW(DT,col1,val1,col2,val2,...,coln,valn) append a single row +% to the DynamicTable +% +% ADDVARARGROW(DT,___,Name,Value) optional 'id' +% +% This function asserts the following: +% 1) The given keyword argument names match one of those ALREADY specified +% by the DynamicTable (that is, colnames MUST be filled out). +% 2) If the dynamic table is non-empty, the types of the column value MUST +% match the keyword value. +% 3) All horizontal data must match the width of the rest of the rows. +% Variable length strings should use cell arrays each row. +% 4) The type of the data cannot be a cell array of numeric values if using +% keyword arguments. For table appending mode, this is how ragged arrays +% are represented. + p = inputParser(); p.KeepUnmatched = true; p.StructExpand = false; From 07a3954c9540c190ee41d0e650e8048c82304ce6 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:04:35 +0200 Subject: [PATCH 2/9] feat: report constant schema violations by context --- +types/+util/checkConstant.m | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/+types/+util/checkConstant.m b/+types/+util/checkConstant.m index 70c298b0f..2bc712609 100644 --- a/+types/+util/checkConstant.m +++ b/+types/+util/checkConstant.m @@ -14,12 +14,25 @@ return end + name = char(name); className = char(className); classNameParts = strsplit(className, '.'); classReference = sprintf('%s', ... className, classNameParts{end}); - error('NWB:Type:ReadOnlyProperty', ... - ['Unable to set the ''%s'' property of class ''%s'' because it ', ... - 'is read-only.'], char(name), classReference) + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:Type:ReadOnlyProperty', ... + sprintf(['The schema requires the ''%s'' property of class ''%s'' ' ... + 'to be %s, but the value is %s.'], ... + name, classReference, formatValue(expectedValue), formatValue(value))) +end + +function str = formatValue(value) + if ischar(value) || (isstring(value) && isscalar(value)) + str = sprintf('''%s''', char(value)); + elseif isnumeric(value) || islogical(value) + str = mat2str(value); + else + str = sprintf('<%s>', class(value)); + end end From 5cd44605867d4eb703a8f3585bee5cf16c330d38 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:05:15 +0200 Subject: [PATCH 3/9] feat: warn on invalid shapes during reads --- +tests/+unit/nwbReadTest.m | 23 +++++++++-------------- +types/+util/validateShape.m | 11 ++++------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/+tests/+unit/nwbReadTest.m b/+tests/+unit/nwbReadTest.m index f186d5cd9..73ddf5030 100644 --- a/+tests/+unit/nwbReadTest.m +++ b/+tests/+unit/nwbReadTest.m @@ -141,20 +141,15 @@ function readFileWithInvalidTimeSeriesTimestampsShape(testCase) fileName = 'invalid_timeseries_timestamps_shape.nwb'; createInvalidTimeSeriesFile(fileName) - try - nwbRead(fileName, "ignorecache"); - testCase.assertFail('Expected nwbRead to fail when timestamps are 2-D.') - catch exception - testCase.verifyEqual(exception.identifier, ... - 'NWB:createParsedType:TypeCreationFailed') - testCase.verifySubstring(exception.message, ... - 'Failed to create object of type "types.core.TimeSeries"') - testCase.verifySubstring(exception.message, ... - 'file location "/acquisition/bad_ts"') - testCase.verifyNotEmpty(exception.cause) - testCase.verifySubstring(exception.cause{1}.message, ... - 'Invalid shape for property "timestamps".') - end + % The file is read permissively: a warning is issued and the + % value is kept, so the data remains accessible (issue #776). + nwb = testCase.verifyWarning(... + @() nwbRead(fileName, "ignorecache"), ... + 'NWB:CheckDims:InvalidDimensions'); + + testCase.verifyClass(nwb, 'NwbFile') + testCase.verifyClass(nwb.acquisition.get('bad_ts'), ... + 'types.core.TimeSeries') end function readWithStringFilenameArg(testCase) diff --git a/+types/+util/validateShape.m b/+types/+util/validateShape.m index 20250281e..bac9afed9 100644 --- a/+types/+util/validateShape.m +++ b/+types/+util/validateShape.m @@ -35,17 +35,14 @@ function validateShape(propertyName, validShapes, value) try types.util.checkDims(valueShape, validShapes, enforceScalarShape); catch MECause - ME = MException(MECause.identifier, ... - 'Invalid shape for property "%s".', propertyName); - ME = ME.addCause(MECause); - + causes = MECause; if isa(value, 'types.untyped.DataPipe') - extraCause = MException('NWB:ValidateShape:InvalidMaxSize', ... + causes(end+1) = MException('NWB:ValidateShape:InvalidMaxSize', ... ['For DataPipe objects, ensure the `maxSize` property ', ... 'matches the valid shape.']); - ME = ME.addCause(extraCause); end - throw(ME) + matnwb.common.validation.reportSchemaViolation(MECause.identifier, ... + sprintf('Invalid shape for property "%s".', propertyName), causes); end % Check actual size of DataPipe and warn if it is not valid From 15e720ebcae70cbe37a56ffcaee8b63fa8c96b0a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:33:44 +0200 Subject: [PATCH 4/9] feat: warn on invalid neurodata types during reads --- .../+unit/+types/+validators/CheckTypeTest.m | 41 +++++++++++++++++++ +types/+util/checkType.m | 8 ++-- 2 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 +tests/+unit/+types/+validators/CheckTypeTest.m diff --git a/+tests/+unit/+types/+validators/CheckTypeTest.m b/+tests/+unit/+types/+validators/CheckTypeTest.m new file mode 100644 index 000000000..1cd02c195 --- /dev/null +++ b/+tests/+unit/+types/+validators/CheckTypeTest.m @@ -0,0 +1,41 @@ +classdef CheckTypeTest < matlab.unittest.TestCase +% CheckTypeTest - Unit tests for types.util.checkType. + + methods (Test) + function testMatchingTypePasses(testCase) + value = types.hdmf_common.VectorData( ... + 'description', 'a column', 'data', (1:3)'); + testCase.verifyWarningFree( ... + @() types.util.checkType( ... + 'col', 'types.hdmf_common.VectorData', value)) + end + + function testWrongTypeErrorsInEditContext(testCase) + value = types.hdmf_common.VectorData( ... + 'description', 'a column', 'data', (1:3)'); + testCase.verifyError( ... + @() types.util.checkType( ... + 'region', 'types.hdmf_common.DynamicTableRegion', value), ... + 'NWB:CheckType:InvalidNeurodataType') + end + + function testWrongTypeWarnsInReadContext(testCase) + value = types.hdmf_common.VectorData( ... + 'description', 'a column', 'data', (1:3)'); + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyWarning( ... + @() types.util.checkType( ... + 'region', 'types.hdmf_common.DynamicTableRegion', value), ... + 'NWB:CheckType:InvalidNeurodataType') + end + + function testUnknownExpectedTypeAlwaysErrors(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyError( ... + @() types.util.checkType('x', 'types.core.NotARealType', 5), ... + 'NWB:CheckType:UnknownNeurodataType') + end + end +end diff --git a/+types/+util/checkType.m b/+types/+util/checkType.m index 58ca1a8f0..b2643f311 100644 --- a/+types/+util/checkType.m +++ b/+types/+util/checkType.m @@ -21,8 +21,10 @@ function checkType(propertyName, expectedType, value) end if ~isa(value, expectedType) - error('NWB:CheckType:InvalidNeurodataType', ... - 'Expected value for property `%s` to be of type `%s`, but got `%s` instead.', ... - propertyName, expectedType, class(value)); + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:CheckType:InvalidNeurodataType', ... + sprintf(['Expected value for property `%s` to be of type `%s`, ' ... + 'but got `%s` instead.'], ... + propertyName, expectedType, class(value))); end end From bc45293fb8ed0eafe1caed2c7cf701859cdd9a65 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:34:18 +0200 Subject: [PATCH 5/9] feat: warn on invalid soft-link targets during reads --- .../+types/+validators/ValidateSoftLinkTest.m | 33 +++++++++++++++++++ +types/+util/validateSoftLink.m | 13 ++++---- 2 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 +tests/+unit/+types/+validators/ValidateSoftLinkTest.m diff --git a/+tests/+unit/+types/+validators/ValidateSoftLinkTest.m b/+tests/+unit/+types/+validators/ValidateSoftLinkTest.m new file mode 100644 index 000000000..a5a07efb6 --- /dev/null +++ b/+tests/+unit/+types/+validators/ValidateSoftLinkTest.m @@ -0,0 +1,33 @@ +classdef ValidateSoftLinkTest < matlab.unittest.TestCase +% ValidateSoftLinkTest - Unit tests for types.util.validateSoftLink. + + methods (Test) + function testWrongTargetTypeErrorsInEditContext(testCase) + softLink = testCase.createSoftLinkWithTargetType('types.core.Device'); + testCase.verifyError( ... + @() types.util.validateSoftLink( ... + 'electrode_group', softLink, 'types.core.ElectrodeGroup'), ... + 'NWB:ValidateSoftLink:InvalidNeurodataType') + end + + function testWrongTargetTypeWarnsInReadContext(testCase) + softLink = testCase.createSoftLinkWithTargetType('types.core.Device'); + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + value = testCase.verifyWarning( ... + @() types.util.validateSoftLink( ... + 'electrode_group', softLink, 'types.core.ElectrodeGroup'), ... + 'NWB:ValidateSoftLink:InvalidNeurodataType'); + testCase.verifyClass(value, 'types.untyped.SoftLink') + end + end + + methods (Access = private) + function softLink = createSoftLinkWithTargetType(testCase, targetType) + testCase.applyFixture( ... + matlab.unittest.fixtures.SuppressedWarningsFixture( ... + 'NWB:SoftLink:DeprecatedPath')); + softLink = types.untyped.SoftLink('', targetType); + end + end +end diff --git a/+types/+util/validateSoftLink.m b/+types/+util/validateSoftLink.m index a6bbc364f..eba9df38d 100644 --- a/+types/+util/validateSoftLink.m +++ b/+types/+util/validateSoftLink.m @@ -23,11 +23,13 @@ if ~isempty(v.target) types.util.checkType(propertyName, targetType, v.target); elseif ~isempty(v.target_type) - assert(types.util.internal.isNameOfA(v.target_type, targetType), ... - 'NWB:ValidateSoftLink:InvalidNeurodataType', ... - ['Expected value for property `%s` to be of type ', ... - '`%s`, but got `%s` instead.'], ... - propertyName, targetType, v.target_type) + if ~types.util.internal.isNameOfA(v.target_type, targetType) + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:ValidateSoftLink:InvalidNeurodataType', ... + sprintf(['Expected value for property `%s` to be ' ... + 'of type `%s`, but got `%s` instead.'], ... + propertyName, targetType, v.target_type)); + end end else types.util.checkType(propertyName, targetType, v); @@ -35,4 +37,3 @@ end end end - From 237385faa15d5427573cd7c5a16dd2b0ef6f1eb2 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:35:03 +0200 Subject: [PATCH 6/9] feat: preserve invalid dtype values during reads --- .../+unit/+types/+validators/CheckDtypeTest.m | 31 ++++++++++ +types/+util/checkDtype.m | 57 +++++++++++++++++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/+tests/+unit/+types/+validators/CheckDtypeTest.m b/+tests/+unit/+types/+validators/CheckDtypeTest.m index 3be8208d7..d9407a375 100644 --- a/+tests/+unit/+types/+validators/CheckDtypeTest.m +++ b/+tests/+unit/+types/+validators/CheckDtypeTest.m @@ -202,6 +202,37 @@ function testAnyDtypeRejectsUnsupportedType(testCase) @() types.util.checkDtype('invalidValue', 'any', {struct()}), ... 'NWB:CheckDType:InvalidType') end + + function testAnyDtypeWarnsForUnsupportedTypeInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + invalidValue = {struct()}; + + value = testCase.verifyWarning( ... + @() types.util.checkDtype('invalidValue', 'any', invalidValue), ... + 'NWB:CheckDType:InvalidType'); + + testCase.verifyEqual(value, invalidValue) + end + + function testStrictModeErrorsInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyError( ... + @() types.util.checkDtype( ... + 'invalidValue', 'any', {struct()}, Mode="strict"), ... + 'NWB:CheckDType:InvalidType') + end + + function testInvalidConversionWarnsInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + invalidValue = single(realmax('single')); + + value = testCase.verifyWarning( ... + @() types.util.checkDtype('precisionLossError', 'uint64', invalidValue), ... + 'NWB:CheckDataType:InvalidConversion'); + + testCase.verifyEqual(value, invalidValue) + end end methods (Static) diff --git a/+types/+util/checkDtype.m b/+types/+util/checkDtype.m index a47b5b24a..869b13dbb 100644 --- a/+types/+util/checkDtype.m +++ b/+types/+util/checkDtype.m @@ -1,4 +1,4 @@ -function value = checkDtype(name, typeDescriptor, value) +function value = checkDtype(name, typeDescriptor, value, options) % checkDtype - Validates and corrects the data type of a given value % % Syntax: @@ -20,13 +20,36 @@ name {mustBeTextScalar} typeDescriptor {mustBeValidTypeDescriptor} value + options.Mode (1,1) string ... + {mustBeMember(options.Mode, ["report", "strict"])} = "report" end + if options.Mode == "strict" + value = checkDtypeStrict(name, typeDescriptor, value, options.Mode); + return + end + + originalValue = value; + try + value = checkDtypeStrict(name, typeDescriptor, value, options.Mode); + catch exception + if isSchemaValidationException(exception) + matnwb.common.validation.reportSchemaViolation( ... + exception.identifier, exception.message, getCauses(exception)); + value = originalValue; + else + rethrow(exception) + end + end +end + +function value = checkDtypeStrict(name, typeDescriptor, value, validationMode) if canBeAnyType(typeDescriptor) value = validateAnyType(value); elseif isstruct(typeDescriptor) % Compound type processing - value = checkDtypeForCompoundDataset(name, typeDescriptor, value); + value = checkDtypeForCompoundDataset( ... + name, typeDescriptor, value, validationMode); elseif isa(value, 'types.untyped.SoftLink') if ~isempty(value.target) @@ -98,6 +121,24 @@ %% Local functions +function tf = isSchemaValidationException(exception) + schemaValidationPrefixes = "NWB:CheckDType:"; + schemaValidationIds = [ ... + "NWB:CheckDataType:InvalidConversion", ... + "NWB:TypeCorrection:InvalidConversion", ... + "NWB:TypeCorrection:PrecisionLossDetected"]; + + tf = startsWith(string(exception.identifier), schemaValidationPrefixes) ... + || any(strcmp(exception.identifier, schemaValidationIds)); +end + +function causes = getCauses(exception) + causes = MException.empty(1, 0); + for iCause = 1:numel(exception.cause) + causes(end+1) = exception.cause{iCause}; %#ok + end +end + function mustBeValidTypeDescriptor(typeDescriptor) isValid = isstruct(typeDescriptor) || ischar(typeDescriptor) || isstring(typeDescriptor); assert( isValid, ... @@ -105,7 +146,8 @@ function mustBeValidTypeDescriptor(typeDescriptor) 'Type descriptor must be a struct, character vector or a string.'); end -function value = checkDtypeForCompoundDataset(name, typeDescriptor, value) +function value = checkDtypeForCompoundDataset( ... + name, typeDescriptor, value, validationMode) valueWrapper = []; if isWrapped(value, typeDescriptor) @@ -138,7 +180,8 @@ function mustBeValidTypeDescriptor(typeDescriptor) if (isstruct(value) && isscalar(value)) || istable(value) % scalar struct or table with columns. - value.(name) = types.util.checkDtype(subName,subType,value.(name)); + value.(name) = types.util.checkDtype(subName, subType, value.(name), ... + Mode=validationMode); elseif isstruct(value) % array of structs for j=1:length(value) @@ -149,11 +192,13 @@ function mustBeValidTypeDescriptor(typeDescriptor) 'NWB:CheckDType:InvalidType',... ['Fields for an array of structs for '... 'compound types should have non-cell scalar values or char arrays.']); - value(j).(name) = types.util.checkDtype(subName, subType, elem); + value(j).(name) = types.util.checkDtype(subName, subType, elem, ... + Mode=validationMode); end else value(expectedFields{iField}) = types.util.checkDtype( ... - subName, subType, value(expectedFields{iField})); + subName, subType, value(expectedFields{iField}), ... + Mode=validationMode); end end From cda555153f901e7da7545eabb0595ec28aa25890 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:35:40 +0200 Subject: [PATCH 7/9] feat: warn after constrained dtype candidates fail --- .../+types/+validators/CheckConstraintTest.m | 32 +++++++++++++++++++ +types/+util/checkConstraint.m | 31 +++++++++++------- 2 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 +tests/+unit/+types/+validators/CheckConstraintTest.m diff --git a/+tests/+unit/+types/+validators/CheckConstraintTest.m b/+tests/+unit/+types/+validators/CheckConstraintTest.m new file mode 100644 index 000000000..5bc350a8f --- /dev/null +++ b/+tests/+unit/+types/+validators/CheckConstraintTest.m @@ -0,0 +1,32 @@ +classdef CheckConstraintTest < matlab.unittest.TestCase +% CheckConstraintTest - Unit tests for types.util.checkConstraint. + + methods (Test) + function testValueMatchingNoConstrainedTypeErrorsInEditContext(testCase) + testCase.verifyError( ... + @() types.util.checkConstraint('group', 'item', struct(), ... + {'types.hdmf_common.VectorData'}, 5), ... + 'NWB:CheckConstraint:InvalidType') + end + + function testValueMatchingNoConstrainedTypeWarnsInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + value = testCase.verifyWarning( ... + @() types.util.checkConstraint('group', 'item', struct(), ... + {'types.hdmf_common.VectorData'}, 5), ... + 'NWB:CheckConstraint:InvalidType'); + testCase.verifyEqual(value, 5) + end + + function testReadContextProbesConstrainedTypesStrictly(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + value = testCase.verifyWarningFree( ... + @() types.util.checkConstraint('group', 'item', struct(), ... + {'types.hdmf_common.VectorData', 'double'}, 5)); + + testCase.verifyEqual(value, 5) + end + end +end diff --git a/+types/+util/checkConstraint.m b/+types/+util/checkConstraint.m index 17f570e44..6c5a04a56 100644 --- a/+types/+util/checkConstraint.m +++ b/+types/+util/checkConstraint.m @@ -12,21 +12,28 @@ for i=1:length(constrained) allowedType = constrained{i}; try - val = types.util.checkDtype([pname '.' name], allowedType, val); + val = types.util.checkDtype( ... + [pname '.' name], allowedType, val, Mode="strict"); return; catch ME - expectedErrorTypes = {... - 'NWB:CheckDType:InvalidType', ... - 'NWB:CheckDType:InvalidShape', ... - 'NWB:CheckDType:InvalidNeurodataType', ... - 'NWB:CheckDataType:InvalidConversion', ... - 'NWB:TypeCorrection:InvalidConversion'}; - if ~any(strcmp(ME.identifier, expectedErrorTypes)) + if ~isExpectedValidationError(ME) rethrow(ME); end end end - error('NWB:CheckConstraint:InvalidType',... - 'Property `%s.%s` should be one of type(s) {%s}. Found type "%s"',... - pname, name, misc.cellPrettyPrint(constrained), class(val)); -end \ No newline at end of file + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:CheckConstraint:InvalidType', ... + sprintf('Property `%s.%s` should be one of type(s) {%s}. Found type "%s"', ... + pname, name, misc.cellPrettyPrint(constrained), class(val))); +end + +function tf = isExpectedValidationError(exception) + expectedPrefixes = "NWB:CheckDType:"; + expectedIds = [ ... + "NWB:CheckDataType:InvalidConversion", ... + "NWB:TypeCorrection:InvalidConversion", ... + "NWB:TypeCorrection:PrecisionLossDetected"]; + + tf = startsWith(string(exception.identifier), expectedPrefixes) ... + || any(strcmp(exception.identifier, expectedIds)); +end From 14d649890d00d1cc972265d716081858cb1b58af Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:37:37 +0200 Subject: [PATCH 8/9] feat: warn on recoverable DynamicTable inconsistencies --- .../+validators/DynamicTableCheckConfigTest.m | 42 +++++++++++++++++++ .../+internal/getOutermostIndexColumnName.m | 9 ++-- +types/+util/+dynamictable/checkConfig.m | 34 +++++++++------ .../+util/+dynamictable/normalizeColnames.m | 2 + 4 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 +tests/+unit/+types/+validators/DynamicTableCheckConfigTest.m diff --git a/+tests/+unit/+types/+validators/DynamicTableCheckConfigTest.m b/+tests/+unit/+types/+validators/DynamicTableCheckConfigTest.m new file mode 100644 index 000000000..0fff393cf --- /dev/null +++ b/+tests/+unit/+types/+validators/DynamicTableCheckConfigTest.m @@ -0,0 +1,42 @@ +classdef DynamicTableCheckConfigTest < matlab.unittest.TestCase +% DynamicTableCheckConfigTest - Unit tests for DynamicTable schema checks. + + methods (Test) + function testInvalidColnamesErrorsInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyError( ... + @() types.hdmf_common.DynamicTable( ... + 'description', 'invalid colnames', 'colnames', 1), ... + 'NWB:DynamicTable:InvalidColumnNames') + end + + function testMismatchedColumnHeightsWarnInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyWarning( ... + @() types.hdmf_common.DynamicTable( ... + 'description', 'mismatched column heights', ... + 'colnames', {'columnA', 'columnB'}, ... + 'id', types.hdmf_common.ElementIdentifiers('data', int64(0:2)), ... + 'columnA', types.hdmf_common.VectorData( ... + 'description', 'column a', 'data', (1:3)'), ... + 'columnB', types.hdmf_common.VectorData( ... + 'description', 'column b', 'data', (1:2)')), ... + 'NWB:DynamicTable:CheckConfig:InvalidShape') + end + + function testMismatchedIdHeightWarnsInReadContext(testCase) + [~, cleanup] = matnwb.common.validation.internal.context("read"); %#ok + + testCase.verifyWarning( ... + @() types.hdmf_common.DynamicTable( ... + 'description', 'mismatched id height', ... + 'colnames', {'columnA'}, ... + 'id', types.hdmf_common.ElementIdentifiers('data', int64(0:1)), ... + 'columnA', types.hdmf_common.VectorData( ... + 'description', 'column a', 'data', (1:3)')), ... + 'NWB:DynamicTable:CheckConfig:InvalidId') + end + end +end diff --git a/+types/+util/+dynamictable/+internal/getOutermostIndexColumnName.m b/+types/+util/+dynamictable/+internal/getOutermostIndexColumnName.m index 552aaa751..0fd794839 100644 --- a/+types/+util/+dynamictable/+internal/getOutermostIndexColumnName.m +++ b/+types/+util/+dynamictable/+internal/getOutermostIndexColumnName.m @@ -17,9 +17,12 @@ if isempty(indexName) return; end - assert(~any(strcmp(columnHistory, indexName)), ... - 'NWB:DynamicTable:CheckConfig:InfiniteReferenceLoop', ... - 'Invalid Table shape detected: There is an infinite loop in your VectorIndex objects.'); + if any(strcmp(columnHistory, indexName)) + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:DynamicTable:CheckConfig:InfiniteReferenceLoop', ... + 'Invalid Table shape detected: There is an infinite loop in your VectorIndex objects.'); + return + end columnHistory{end+1} = indexName; %#ok outermostIndexColumnName = indexName; end diff --git a/+types/+util/+dynamictable/checkConfig.m b/+types/+util/+dynamictable/checkConfig.m index a170c2179..32ef6aaa4 100644 --- a/+types/+util/+dynamictable/checkConfig.m +++ b/+types/+util/+dynamictable/checkConfig.m @@ -65,9 +65,12 @@ function checkConfig(DynamicTable, ignoreList) DynamicTable, columns{iCol}); columnHeight = unique(columnHeight); - assert(isscalar(columnHeight), ... - 'NWB:DynamicTable:CheckConfig:InvalidShape', ... - 'Invalid compound column detected: compound column heights must all be the same.'); + if ~isscalar(columnHeight) + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:DynamicTable:CheckConfig:InvalidShape', ... + 'Invalid compound column detected: compound column heights must all be the same.'); + return + end columnHeights(iCol) = columnHeight; columnNames(iCol) = columnName; end @@ -78,11 +81,15 @@ function checkConfig(DynamicTable, ignoreList) end formatSpec = sprintf(' %%-%ds %%d', max(strlength(columnNames))); - assert(isscalar(tableHeight), ... - 'NWB:DynamicTable:CheckConfig:InvalidShape', ... - ['Invalid table: all columns must have the same height (number of rows).\n\n' ... - 'Detected column heights:\n' ... - strjoin( compose(formatSpec, columnNames, columnHeights), newline) ]); + if ~isscalar(tableHeight) + detectedHeightsText = strjoin( ... + compose(formatSpec, columnNames, columnHeights), newline); + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:DynamicTable:CheckConfig:InvalidShape', ... + sprintf(['Invalid table: all columns must have the same height (number of rows).\n\n', ... + 'Detected column heights:\n%s'], detectedHeightsText)); + return + end if isempty(DynamicTable.id) types.util.dynamictable.internal.initDynamicTableId(DynamicTable, tableHeight); @@ -90,10 +97,13 @@ function checkConfig(DynamicTable, ignoreList) end numIds = types.util.dynamictable.internal.getColumnHeight(DynamicTable.id); - assert(tableHeight == numIds, ... - 'NWB:DynamicTable:CheckConfig:InvalidId', ... - 'Special column `id` of DynamicTable needs to match the detected height of %d. Found %d IDs.', ... - tableHeight, numIds); + if tableHeight ~= numIds + matnwb.common.validation.reportSchemaViolation( ... + 'NWB:DynamicTable:CheckConfig:InvalidId', ... + sprintf(['Special column `id` of DynamicTable needs to match ', ... + 'the detected height of %d. Found %d IDs.'], tableHeight, numIds)); + return + end end function names = getDetectedColumnNames(DynamicTable) diff --git a/+types/+util/+dynamictable/normalizeColnames.m b/+types/+util/+dynamictable/normalizeColnames.m index 358e0d59c..d37836825 100644 --- a/+types/+util/+dynamictable/normalizeColnames.m +++ b/+types/+util/+dynamictable/normalizeColnames.m @@ -5,6 +5,8 @@ return end + % Non-text column names cannot be used to look up columns, so they are a + % hard error in every validation context rather than a warn-on-read issue. assert(iscellstr(colnames) || isstring(colnames) || ischar(colnames), ... 'NWB:DynamicTable:InvalidColumnNames', ... 'Column names must be a cell array of character vectors.'); From 6d1fda04f6950d5b9db93ac37391f72b99477d06 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 16:38:03 +0200 Subject: [PATCH 9/9] test: cover permissive dtype validation during parsing --- +tests/+unit/+io/testCreateParsedType.m | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/+tests/+unit/+io/testCreateParsedType.m b/+tests/+unit/+io/testCreateParsedType.m index 88618e14f..2916f05dc 100644 --- a/+tests/+unit/+io/testCreateParsedType.m +++ b/+tests/+unit/+io/testCreateParsedType.m @@ -37,20 +37,16 @@ function testCreateTypeWithInvalidInputs(testCase) end function testCreateTypeWithInvalidPropertyValue(testCase) + % Invalid property values warn and remain accessible during reads. testPath = 'some/dataset/path'; testType = 'types.hdmf_common.VectorIndex'; kwargs = {'data', 'text is not numeric indices'}; - try - io.createParsedType(testPath, testType, kwargs{:}); - testCase.verifyFail('Expected io.createParsedType to throw an error.'); - catch exception - testCase.verifyEqual(... - exception.identifier, ... - 'NWB:createParsedType:TypeCreationFailed'); - testCase.verifyNotEmpty(strfind(exception.message, testType)); - testCase.verifyNotEmpty(strfind(exception.message, testPath)); - end + type = testCase.verifyWarning( ... + @() io.createParsedType(testPath, testType, kwargs{:}), ... + 'NWB:CheckDataType:InvalidConversion'); + + testCase.verifyClass(type, testType) end function testCreateDynamicTableWithDuplicateColnamesWarns(testCase)