Refactor dynamic table

+tests/+sanity/GenerationTest.m

@@ -51,7 +51,7 @@ function dynamicTableMethodsTest(testCase)
                 '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" ...
+                'NWB:DynamicTable:AddRow:TableIsUnsupported' ...
             );
 
             retrievalIndex = round(1 + 4 .* rand(10, 1));

+tests/+system/DynamicTableTest.m

@@ -105,25 +105,22 @@ function addContainerUndefinedIDs(~, file)
             c = file.intervals_trials.vectordata.get('randomvalues');
         end
 
-        function appendContainer(testCase, file)
-            container = testCase.getContainer(file);
+        function appendContainer(testCase, File)
+            container = testCase.getContainer(File);
             container.data = rand(size(container.data)); % new random values.
-            file.intervals_trials.vectordata.get('stringdata').data = repmat({'FALSE'}, 20, 1);
-            %test adding new column with argument
-            file.intervals_trials.addColumn( ...
-                'newcolumn', types.hdmf_common.VectorData( ...
-                'description', 'newly added column', ...
-                'data', (20:-1:1) .' ...
-                ) ...
-                );
+            File.intervals_trials.vectordata.get('stringdata').data = repmat({'FALSE'}, 20, 1);
+            % test adding new column with argument
+
+            Column = types.hdmf_common.VectorData( ...
+                'description', 'newly added column' ...
+                , 'data', (20:-1:1) .');
+            File.intervals_trials.addColumn('newcolumn', Column);
             % 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" ...
-                );
+            Table = table( ...
+                (1:2:40)', (1:4:80)' ...
+                , 'VariableNames', {'newcolumn2', 'newcolumn3'});
+            testCase.verifyError(@()File.intervals_trials.addColumn(Table) ...
+                , 'NWB:DynamicTable:AddColumn:InvalidArgument');
         end
 
         function appendRaggedContainer(~, file)
@@ -138,12 +135,9 @@ function appendRaggedContainer(~, file)
                 startInd = endInd+1;
             end
             % get corresponding VectorData and VectorIndex
-            [rag_col, rag_col_index] = util.create_indexed_column(dataArray);
+            [Column, Index] = util.create_indexed_column(dataArray);
             % append ragged column
-            file.intervals_trials.addColumn( ...
-                'newraggedcolumn',rag_col, ...
-                'newraggedcolumn_index',rag_col_index ...
-                )
+            file.intervals_trials.addColumn('newraggedcolumn', Column, Index);
         end
     end
 

+tests/+unit/dataPipeTest.m

@@ -64,61 +64,59 @@ function testInit(testCase)
 function testIndex(testCase)
     filename = 'testIndexing.h5';
     name = '/test_data';
-    
+
     data = rand(100, 100, 100);
     Pipe = types.untyped.DataPipe('data', data);
-    
+
     testCase.verifyEqual(Pipe(:), data(:));
     testCase.verifyEqual(Pipe(:,:,1), data(:,:,1));
-    
+
     fid = H5F.create(filename);
     Pipe.export(fid, name, {}); % bind the pipe.
     H5F.close(fid);
-    
+
     testCase.verifyEqual(Pipe(:), data(:));
     testCase.verifyEqual(Pipe(:,:,1), data(:,:,1));
 end
 
 function testAppend(testCase)
     filename = 'testIterativeWrite.h5';
-
     Pipe = types.untyped.DataPipe(...
         'maxSize', [10 13 15],...
         'axis', 3,...
         'chunkSize', [10 13 1],...
         'dataType', 'uint8',...
         'compressionLevel', 5);
-    
+
     OneDimensionPipe = types.untyped.DataPipe('maxSize', Inf, 'data', [7, 8, 9]);
-    
+
     %% create test file
     fid = H5F.create(filename);
-    
+
     initialData = createData(Pipe.dataType, [10 13 10]);
     Pipe.internal.data = initialData;
     Pipe.export(fid, '/test_data', {}); % bind
     OneDimensionPipe.export(fid, '/test_one_dim_data', {});
-    
+
     H5F.close(fid);
-    
+
     %% append data
     totalLength = 3;
     appendData = zeros([10 13 totalLength], Pipe.dataType);
     for i = 1:totalLength
         appendData(:,:,i) = createData(Pipe.dataType, Pipe.chunkSize);
         Pipe.append(appendData(:,:,i));
     end
-    
+
     for i = 1:totalLength
         OneDimensionPipe.append(rand());
     end
-    
+
     %% verify data
     Pipe = types.untyped.DataPipe('filename', filename, 'path', '/test_data');
     readData = Pipe.load();
     testCase.verifyEqual(readData(:,:,1:10), initialData);
     testCase.verifyEqual(readData(:,:,11:end), appendData);
-
     OneDimensionPipe = types.untyped.DataPipe('filename', filename, 'path', '/test_one_dim_data');
     readData = OneDimensionPipe.load();
     testCase.verifyTrue(isvector(readData));
@@ -130,30 +128,30 @@ function testExternalFilters(testCase)
     import types.untyped.datapipe.dynamic.Filter;
     import types.untyped.datapipe.properties.DynamicFilter;
     import types.untyped.datapipe.properties.Shuffle;
-    
+
     testCase.assumeTrue(logical(H5Z.filter_avail(uint32(Filter.LZ4))));
-    
+
     filename = 'testExternalWrite.h5';
-    
+
     Pipe = types.untyped.DataPipe(...
         'maxSize', [10 13 15],...
         'axis', 3,...
         'chunkSize', [10 13 1],...
         'dataType', 'uint8',...
         'filters', [Shuffle() DynamicFilter(Filter.LZ4)]);
-    
+
     OneDimensionPipe = types.untyped.DataPipe('maxSize', Inf, 'data', [7, 8, 9]);
-    
+
     %% create test file
     fid = H5F.create(filename);
-    
+
     initialData = createData(Pipe.dataType, [10 13 10]);
     Pipe.internal.data = initialData;
     Pipe.export(fid, '/test_data', {}); % bind
     OneDimensionPipe.export(fid, '/test_one_dim_data', {});
-    
+
     H5F.close(fid);
-    
+
     %% append data
     totalLength = 3;
     appendData = zeros([10 13 totalLength], Pipe.dataType);

+types/+util/+dynamictable/addColumn.m

@@ -1,41 +1,43 @@
 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
+    % ADDCOLUMN Given a dynamic table and a keyword argument, add a column to the dynamic table.
+    %
+    %  ADDCOLUMN(DT,NM,VD)
+    %  append specified column name NM and non-ragged VectorData VD to DynamicTable DT
+    %
+    %  ADDCOLUMN(DT,NM, VD, VI) append specified column by col_name NM represented
+    %  by multiple VectorIndex references VI ordered in such a way where VI(n) references V(n-1) and
+    %  VI(1) references VectorData VD.
+    %
+    % 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'});
+    validateattributes(DynamicTable ...
+        , {'types.core.DynamicTable', 'types.hdmf_common.DynamicTable'} ...
+        , {'scalar'});
 
-assert(nargin > 1, 'NWB:DynamicTable:AddColumn:NoData', 'Not enough arguments');
+    assert(nargin > 1, 'NWB:DynamicTable:AddColumn:NoData', 'Not enough arguments');
 
-if isempty(DynamicTable.id)
-    if 8 == exist('types.hdmf_common.ElementIdentifiers', 'class')
-        DynamicTable.id = types.hdmf_common.ElementIdentifiers();
-    else % legacy Element Identifiers
-        DynamicTable.id = types.core.ElementIdentifiers();
+    if isempty(DynamicTable.id)
+        if 8 == exist('types.hdmf_common.ElementIdentifiers', 'class')
+            DynamicTable.id = types.hdmf_common.ElementIdentifiers();
+        else % legacy Element Identifiers
+            DynamicTable.id = types.core.ElementIdentifiers();
+        end
     end
-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.']);
+    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
+    assert(~istable(varargin{1}) ...
+        , 'NWB:DynamicTable:AddColumn:InvalidArgument' ...
+        , [ ...
+        'Using MATLAB tables as input to the addColumn DynamicTable method has been deprecated. ' ...
+        'Please, use key-value pairs instead.']);
     types.util.dynamictable.addVarargColumn(DynamicTable, varargin{:});
 end

+types/+util/+dynamictable/addRawData.m

@@ -1,25 +1,25 @@
-function addRawData(DynamicTable, column, data)
+function addRawData(DynamicTable, columnName, data)
     %ADDRAWDATA Internal method for adding data to DynamicTable given column
     % name and data. Indices are determined based on data format and available
     % indices.
-    validateattributes(column, {'char'}, {'scalartext'});
+    validateattributes(columnName, {'char'}, {'scalartext'});
 
-    if (isprop(DynamicTable, column) && isempty(DynamicTable.(column))) ...
-            || (~isprop(DynamicTable, column) && ~isKey(DynamicTable.vectordata, column))
+    if (isprop(DynamicTable, columnName) && isempty(DynamicTable.(columnName))) ...
+            || (~isprop(DynamicTable, columnName) && ~isKey(DynamicTable.vectordata, columnName))
         % No vecdata found anywhere. Initialize.
-        initVecData(DynamicTable, column, class(data));
+        initVecData(DynamicTable, columnName, class(data));
     end
 
-    if isprop(DynamicTable, column)
-        Vector = DynamicTable.(column);
-    elseif isprop(DynamicTable, 'vectorindex') && DynamicTable.vectorindex.isKey(column)
-        Vector = DynamicTable.vectorindex.get(column);
+    if isprop(DynamicTable, columnName)
+        Vector = DynamicTable.(columnName);
+    elseif isprop(DynamicTable, 'vectorindex') && DynamicTable.vectorindex.isKey(columnName)
+        Vector = DynamicTable.vectorindex.get(columnName);
     else
-        Vector = DynamicTable.vectordata.get(column);
+        Vector = DynamicTable.vectordata.get(columnName);
     end
 
     % grab all available indices for column.
-    indexChain = {column};
+    indexChain = {columnName};
     while true
         index = types.util.dynamictable.getIndex(DynamicTable, indexChain{end});
         if isempty(index)
@@ -35,10 +35,11 @@
 
     % find true nesting depth of column data.
     if isa(Vector.data, 'types.untyped.DataPipe')
-        depth = getNestedDataDepth(data, 'dataPipeDimension', Vector.data.axis);
+        depthRequestOptions = {'dataPipeDimension', Vector.data.axis};
     else
-        depth = getNestedDataDepth(data);
+        depthRequestOptions = {};
     end
+    depth = types.util.dynamictable.getDataDepth(data, depthRequestOptions{:});
 
     % add indices until it matches depth.
     for iVec = (length(indexChain)+1):depth
@@ -99,35 +100,6 @@
     end
 end
 
-function depth = getNestedDataDepth(data, varargin)
-    p = inputParser;
-    p.addParameter('dataPipeDimension', [], @(x)isnumeric(x) && (isempty(x) || isscalar(x)));
-    p.parse(varargin{:});
-
-    depth = 1;
-    subData = data;
-    while iscell(subData) && ~iscellstr(subData)
-        depth = depth + 1;
-        subData = subData{1};
-    end
-
-    % special case where the final data is in fact multiple rows to begin
-    % with.
-    if isempty(p.Results.dataPipeDimension)
-        if ischar(subData)
-            isMultiRow = 1 < size(subData, 1);
-        else
-            isMultiRow = (ismatrix(subData) && 1 < size(subData, 2)) ...
-                || (isvector(subData) && 1 < length(subData));
-        end
-    else
-        isMultiRow = 1 < size(subData, p.Results.dataPipeDimension);
-    end
-    if isMultiRow
-        depth = depth + 1;
-    end
-end
-
 function numRows = nestedAdd(DynamicTable, indChain, data)
     name = indChain{1};