Skip to content

Commit

Permalink
Merge pull request #598 from NeurodataWithoutBorders/597-nwb-to-table…
Browse files Browse the repository at this point in the history
…-bug

Fix: bug in nwbToTable (Issue #597 )
  • Loading branch information
ehennestad authored Sep 30, 2024
2 parents c4bb19c + 7787ff2 commit bc4b87d
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 8 deletions.
72 changes: 72 additions & 0 deletions +tests/+unit/dynamicTableTest.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
function tests = dynamicTableTest()
tests = functiontests(localfunctions);
end

function setupOnce(testCase)
rootPath = fullfile(fileparts(mfilename('fullpath')), '..', '..');
testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));
testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture);
generateCore('savedir', '.');
rehash();
end

function setup(testCase) %#ok<INUSD>
% pass
end

function testNwbToTableWithReferencedTablesAsRowIndices(testCase)
% The default mode for the toTable() method is to return the row indices
% for dynamic table regions. This test verifies that the data type of
% the converted table columns is int64, the default type for indices.
dtr_table = createDynamicTableWithTableRegionReferences();
convertedTable = dtr_table.toTable();

testCase.verifyClass(convertedTable.dtr_col_a(1), 'int64')
testCase.verifyClass(convertedTable.dtr_col_b(1), 'int64')
end

function testNwbToTableWithReferencedTablesAsTableRows(testCase)
% An alternative mode for the toTable() method is to return the referenced
% table rows for dynamic table regions as subtables. This test verifies that
% the data type of the converted table columns is table.
dtr_table = createDynamicTableWithTableRegionReferences();
convertedTable = dtr_table.toTable(false); % Return

row1colA = convertedTable.dtr_col_a(1);
row1colB = convertedTable.dtr_col_b(1);
if iscell(row1colA); row1colA = row1colA{1}; end
if iscell(row1colB); row1colB = row1colB{1}; end

testCase.verifyClass(row1colA, 'table')
testCase.verifyClass(row1colB, 'table')
end

% Non-test functions
function dtr_table = createDynamicTableWithTableRegionReferences()
% Create a dynamic table with two columns, where the data of each column is
% a dynamic table region referencing another dynamic table.
T = table([1;2;3], {'a';'b';'c'}, 'VariableNames', {'col1', 'col2'});
T.Properties.VariableDescriptions = {'column #1', 'column #2'};

T = util.table2nwb(T);

dtr_col_a = types.hdmf_common.DynamicTableRegion( ...
'description', 'references multiple rows of earlier table', ...
'data', [0; 1; 1; 0], ... # 0-indexed
'table',types.untyped.ObjectView(T) ... % object view of target table
);

dtr_col_b = types.hdmf_common.DynamicTableRegion( ...
'description', 'references multiple rows of earlier table', ...
'data', [1; 2; 2; 1], ... # 0-indexed
'table',types.untyped.ObjectView(T) ... % object view of target table
);

dtr_table = types.hdmf_common.DynamicTable( ...
'description', 'test table with DynamicTableRegion', ...
'colnames', {'dtr_col_a', 'dtr_col_b'}, ...
'dtr_col_a', dtr_col_a, ...
'dtr_col_b', dtr_col_b, ...
'id',types.hdmf_common.ElementIdentifiers('data', [0; 1; 2; 3]) ...
);
end
18 changes: 10 additions & 8 deletions +types/+util/+dynamictable/nwbToTable.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
%NWBTOTABLE converts from a NWB DynamicTable to a MATLAB table
%
% MATLABTABLE = NWBTOTABLE(T) converts object T of class types.core.DynamicTable
% into a MATLAB Tale
% into a MATLAB Table
%
% MATLABTABLE = NWBTOTABLE(T, INDEX) If INDEX is FALSE, includes rows referenced by a
% DynamicTableRegion as nested subtables
Expand Down Expand Up @@ -50,10 +50,10 @@
);

% deal with DynamicTableRegion columns when index is false
columns = DynamicTable.colnames;
i = 1;
while i < length(columns)
cn = DynamicTable.colnames{i};
[columns, remainingColumns] = deal(DynamicTable.colnames);

for i = 1:length(columns)
cn = columns{i};
if isprop(DynamicTable, cn)
cv = DynamicTable.(cn);
elseif isprop(DynamicTable, 'vectorindex') && DynamicTable.vectorindex.isKey(cn) % Schema version < 2.3.0
Expand All @@ -71,15 +71,17 @@
cv{r,1} = ref_table.getRow(row_idxs(r)+1);
end
matlabTable.(cn) = cv;
columns(i) = [];
remainingColumns = setdiff(remainingColumns, cn, 'stable');
else
i = i+1;
% pass
end
end
% append remaining columns to table
% making the assumption that length of ids reflects table height
matlabTable = [matlabTable DynamicTable.getRow( ...
1:length(ids), ...
'columns', columns ...
'columns', remainingColumns ...
)];

% Update the columns order to be the same as the original
matlabTable = matlabTable(:, [{'id'}, columns]);

0 comments on commit bc4b87d

Please sign in to comment.