Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore check for DynamicTable shape #385

Merged
merged 7 commits into from
Jan 13, 2022
Merged

restore check for DynamicTable shape #385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fa76bf1
restoring dimension check and comparison with id length
Jan 7, 2022
5e33eed
removed addRows from DynamicTable test
Jan 7, 2022
84a3092
remove band-aid solution to addRow, empty id will be taken care of by…
Jan 10, 2022
767aa9b
added checkConfig tests
Jan 10, 2022
c640563
Merge branch 'master' of https://github.com/cechava/matnwb into resto…
Jan 12, 2022
06bc492
updated tests for expandable tables
Jan 12, 2022
b7a40e3
small modification to expandable table test
Jan 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 51 additions & 32 deletions +tests/+system/DynamicTableTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ function addContainer(~, file)
'stringdata', str_col, ...
'id', types.hdmf_common.ElementIdentifiers('data', ids(1:nrows)) ...
);
% check table configuration.
% This can be removed when addRow is added back in
types.util.dynamictable.checkConfig(file.intervals_trials)
end

function addExpandableDynamicTable(~, file, start_array, stop_array, ...
Expand All @@ -61,40 +64,37 @@ function addExpandableDynamicTable(~, file, start_array, stop_array, ...
start_time_exp = types.hdmf_common.VectorData( ...
'description', 'start times', ...
'data', types.untyped.DataPipe( ...
'data', start_array', ...
'maxSize', [Inf, 1], ...
'axis', 1 ...
'data', start_array, ...
'maxSize', Inf ...
) ...
);
stop_time_exp = types.hdmf_common.VectorData( ...
'description', 'stop times', ...
'data', types.untyped.DataPipe( ...
'data', stop_array', ...
'maxSize', [Inf, 1], ...
'axis', 1 ...
'data', stop_array, ...
'maxSize', Inf ...
) ...
);
random_exp = types.hdmf_common.VectorData( ...
'description', 'random data column', ...
'data', types.untyped.DataPipe( ...
'data', random_array, ...
'maxSize', [Inf, 1], ...
'axis', 1 ...
'data', random_array', ...
'maxSize', [1, Inf], ...
'axis', 2 ...
)...
);
random_multi_exp = types.hdmf_common.VectorData( ...
'description', 'random data column', ...
'data', types.untyped.DataPipe( ...
'data', random_multi_array, ...
'maxSize', [Inf, 3 , 2], ...
'axis', 1 ...
'maxSize', [3 , 2, Inf], ...
'axis', 3 ...
)...
);
ids_exp = types.hdmf_common.ElementIdentifiers( ...
'data', types.untyped.DataPipe( ...
'data', id_array', ...
'maxSize', [Inf, 1], ...
'axis', 1 ...
'data', id_array, ...
'maxSize', Inf ...
) ...
);
% create expandable table
Expand All @@ -110,7 +110,10 @@ function addExpandableDynamicTable(~, file, start_array, stop_array, ...
'randomvalues', random_exp, ...
'random_multi', random_multi_exp, ...
'id', ids_exp ...
);
);
% check table configuration.
% This can be removed when addRow is added back in
types.util.dynamictable.checkConfig(file.intervals_trials)
end

function addContainerUnevenColumns(~, file)
Expand All @@ -121,7 +124,7 @@ function addContainerUnevenColumns(~, file)
'colnames', colnames, ...
'start_time', types.hdmf_common.VectorData( ...
'description', 'start time column', ...
'data', (1:20)' ...
'data', (1:5)' ...
), ...
'stop_time', types.hdmf_common.VectorData( ...
'description', 'stop time column', ...
Expand Down Expand Up @@ -286,33 +289,25 @@ function ExpandableTableTest(testCase)
stop_time_array = start_time_array + 1;
rng(1); % to be able replicate random values
random_val_array = rand(nrows, 1);
random_multi_array = rand(nrows, 3, 2);
random_multi_array = rand(3, 2, nrows);
% create expandable table with first half of arrays
testCase.addExpandableDynamicTable(testCase.file, ...
start_time_array(1:10), stop_time_array(1:10), ...
random_val_array(1:10), random_multi_array(1:10, : ,:), ...
random_val_array(1:10), random_multi_array(:, : ,1:10), ...
id(1:10));
% export and read-in expandable table
filename = ['MatNWB.' testCase.className() '.ExpandableTableTest.nwb'];
nwbExport(testCase.file, filename);
readFile = nwbRead(filename, 'ignorecache');
% add rows to expandable table and export
for i = 11:20
readFile.intervals_trials.addRow( ...
'start_time', start_time_array(i), ...
'stop_time', stop_time_array(i), ...
'randomvalues', random_val_array(i), ...
'random_multi', random_multi_array(i, :, :), ...
'id', id(i) ...
)
end
nwbExport(readFile, filename)
% removing addRows test until function is updated
% read in expanded table
readFile = nwbRead(filename, 'ignorecache');
% test getRow
actualData = readFile.intervals_trials.getRow(1:20, ...
actualData = readFile.intervals_trials.getRow(1:10, ...
'columns', {'randomvalues'});
testCase.verifyEqual(random_val_array, actualData.randomvalues);
testCase.verifyEqual( ...
random_val_array(1:10), ...
actualData.randomvalues ...
);
end

function toTableTest(testCase)
Expand Down Expand Up @@ -366,6 +361,30 @@ function toTableTest(testCase)
);
end
end
function DynamicTableCheckTest(testCase)
% Verify that the checkConfig utility function
% throws error when defining an invalid table
%
% 1. Defining a table with columns of unmatched length
testCase.verifyError( ...
@() testCase.addContainerUnevenColumns(testCase.file), ...
"NWB:DynamicTable" ...
)
% 2. Defining a table with length of id's does not match
% the number of columns
testCase.verifyError( ...
@() testCase.addContainerUnmatchedIDs(testCase.file), ...
"NWB:DynamicTable" ...
)
%3. Defining a table with unspecified IDs
testCase.addContainerUndefinedIDs(testCase.file)
Table = testCase.file.intervals_trials;
% verify created IDs of same length as columns
expectedLength = length(Table.start_time.data);
actualLength = length(Table.id.data);
testCase.verifyEqual(expectedLength, actualLength)

end
end
end
function zipped = zipArrays(A,B)
Expand Down
4 changes: 0 additions & 4 deletions +types/+util/+dynamictable/addRow.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ function addRow(DynamicTable, varargin)

if 8 == exist('types.hdmf_common.ElementIdentifiers', 'class')
types.util.dynamictable.checkConfig(DynamicTable)
% for bandaid solution
if isempty(DynamicTable.id)
DynamicTable.id = types.hdmf_common.ElementIdentifiers();
end
else % legacy Element Identifiers
if isempty(DynamicTable.id)
DynamicTable.id = types.core.ElementIdentifiers();
Expand Down
111 changes: 111 additions & 0 deletions +types/+util/+dynamictable/checkConfig.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,117 @@ function checkConfig(DynamicTable, varargin)
DynamicTable.colnames = removeNulls(DynamicTable.colnames);
end
end
% do not check specified columns - useful for classes that build on DynamicTable class
columns = setdiff(DynamicTable.colnames,ignoreList);
% keep track of last non-ragged column index; to prevent looping over array twice
c = 1;
lastStraightCol = 0;
lengths = zeros(length(columns),1);
while c <= length(columns)
found_cv = 0; %reset flag
cn = columns{c};
% ignore columns that have an index (i.e. ragged), length will be unmatched
indexName = types.util.dynamictable.getIndex(DynamicTable, cn);
if isempty(indexName)
% retrieve data vector
if isprop(DynamicTable, cn)
cv = DynamicTable.(cn);
found_cv = 1;
else
if ~isempty(keys(DynamicTable.vectordata))
try
cv = DynamicTable.vectordata.get(cn);
found_cv = 1;
catch % catch legacy table instance
cv = DynamicTable.vectorindex.get(cn);
found_cv = 1;
end
end
end
if found_cv && ~isempty(cv)
% figure out vector height
if isa(cv.data,'types.untyped.DataStub')
colHeight = cv.data.dims(end);
elseif isa(cv.data,'types.untyped.DataPipe')
if length(cv.data.internal.maxSize) == 1
% catch 1D column
rank = 1;
elseif ismatrix(cv.data.internal.maxSize) && ...
cv.data.internal.maxSize(2) == 1
% catch column vector
rank = 1;
else
rank = length(cv.data.internal.maxSize);
end
selectInd = cell(1, rank);
selectInd(1:end) = {':'};
colHeight = size(cv.data(selectInd{:}),rank);
else
if iscolumn(cv.data)
%catch column vector
colHeight = length(cv.data);
else
colHeight = size(cv.data,ndims(cv.data));% interested in last dimension
end
end
lengths(c) = colHeight;
end
if lastStraightCol > 0 && any(lengths>0)
if min(lengths(lengths>0)) > 1
% skip assertion if we can infer this a table with single row.
% In that case, multidimensional columns can be ambiguous
assert(lengths(c)==lengths(lastStraightCol), ...
'NWB:DynamicTable', ...
'All columns must be the same length.' ...
);
end
end
lastStraightCol = c;
else
if ~any(strcmp(columns,indexName))
columns{length(columns)+1} = indexName;
end
end
c = c+1;
end
if ~isempty(lengths)
if isempty(DynamicTable.id) || isempty(DynamicTable.id.data(:))
if 8 == exist('types.hdmf_common.ElementIdentifiers', 'class')
DynamicTable.id = types.hdmf_common.ElementIdentifiers( ...
'data', int64((1:min(lengths(lengths>0)))-1)' ...
);
else % legacy Element Identifiers
DynamicTable.id = types.core.ElementIdentifiers( ...
'data', int64((1:min(lengths(lengths>0)))-1)' ...
);
end
else
if lastStraightCol > 0 && any(lengths>0)
if min(lengths(lengths>0)) > 1 && ...
length(lengths)>1
% skip assertion if we can infer this a table with single row.
% In that case, multidimensional columns can be ambiguous
assert(lengths(lastStraightCol) == length(DynamicTable.id.data(:)), ...
'NWB:DynamicTable', ...
'Must provide same number of ids as length of columns.' ...
);
elseif length(lengths)==1 && ...
length(DynamicTable.id.data(:)) ~= lengths(lastStraightCol)
% skip if single column and id height matches column height
assert(length(DynamicTable.id.data(:))==1, ... % single-entry case
'NWB:DynamicTable', ...
'Must provide same number of ids as length of columns.' ...
);
end
end
end
else
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
function in = removeNulls(in)
in(double(in) == 0) = [];
Expand Down