Skip to content

Commit cf16d37

Browse files
lawrence-mbflawrence-mbf
authored
Merge pull request #461 from NeurodataWithoutBorders/458-prevent-type-erasure-on-add-row
Cast empty array to correct type before adding row
2 parents 595d94d + 07d929e commit cf16d37

26 files changed

+297
-371
lines changed

‎+file/mapType.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
return;
2727
end
2828

29-
assert(ischar(dtype), 'MatNWB:MapType:InvalidDtype', ...
29+
assert(ischar(dtype), 'NWB:MapType:InvalidDtype', ...
3030
'schema attribute `dtype` returned in unsupported type `%s`', class(dtype));
3131

3232
switch dtype

‎+io/getRefData.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
try
1010
did = H5D.open(fid, refpaths{i});
1111
catch ME
12-
error('MatNWB:getRefData:InvalidPath',...
12+
error('NWB:getRefData:InvalidPath',...
1313
'Reference path `%s` was invalid', refpaths{i});
1414
end
1515
sid = H5D.get_space(did);
@@ -29,7 +29,7 @@
2929
try
3030
ref_data(:, i) = H5R.create(fid, ref(i).path, ref(i).reftype, refspace(i));
3131
catch ME
32-
error('MatNWB:getRefData:InvalidPath',...
32+
error('NWB:getRefData:InvalidPath',...
3333
'Reference path `%s` was invalid', ref(i).path);
3434
end
3535
if H5I.is_valid(refspace(i))

‎+io/parseAttributes.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
% MATLAB can't have arbitrary enum values.
5555
attributeValue = strcmp('TRUE', attr.Value);
5656
else
57-
warning('MatNWB:Attribute:UnknownEnum', ...
57+
warning('NWB:Attribute:UnknownEnum', ...
5858
['Encountered unknown enum under field `%s` with %d members. ' ...
5959
'Will be saved as cell array of characters.'], ...
6060
attr.Name, length(attr.Datatype.Type.Member));

‎+io/parseDataset.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
if io.isBool(datatype.Type)
4646
data = strcmp('TRUE', data);
4747
else
48-
warning('MatNWB:Dataset:UnknownEnum', ...
48+
warning('NWB:Dataset:UnknownEnum', ...
4949
['Encountered unknown enum under field `%s` with %d members. ' ...
5050
'Will be saved as cell array of characters.'], ...
5151
info.Name, length(datatype.Type.Member));

‎+tests/+system/DynamicTableTest.m‎

Lines changed: 34 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -2,146 +2,37 @@
22
methods
33
function addContainer(~, file)
44
colnames = {'start_time', 'stop_time', 'randomvalues', ...
5-
'random_multi', 'stringdata', 'compound_data', 'compound_data_struct'};
5+
'random_multi', 'stringdata', 'compound_table', 'compound_struct', 'logical_data'};
66
%add trailing nulls to columnames
77
for c =1:length(colnames)
88
colnames{c} = char([double(colnames{c}) zeros(1,randi(10))]);
99
end
10-
% until addRow works with multidimensional matrices, define array in one go
10+
1111
nrows = 20;
1212
ids = primes(100)';
13-
start_col = types.hdmf_common.VectorData( ...
14-
'description', 'start_times column', ...
15-
'data', (1:nrows)' ...
16-
);
17-
stop_col = types.hdmf_common.VectorData( ...
18-
'description', 'stop_times column', ...
19-
'data', (1:nrows)'+1 ...
20-
);
21-
rv_col = types.hdmf_common.VectorData( ...
22-
'description', 'randomvalues column', ...
23-
'data', rand(8*nrows,1) ...
24-
);
25-
rv_index = zipArrays(5:8:8*nrows, 8:8:8*nrows);
26-
rv_index_col = types.hdmf_common.VectorIndex( ...
27-
'description', 'index into randomvalues column', ...
28-
'target',types.untyped.ObjectView(rv_col), ...
29-
'data', rv_index ...
30-
);
31-
rv_index_index = 2:2:2*nrows;
32-
rv_index_index_col = types.hdmf_common.VectorIndex( ...
33-
'description', 'index into randomvalues_index column', ...
34-
'target',types.untyped.ObjectView(rv_index_col), ...
35-
'data', rv_index_index ...
36-
);
37-
multi_col = types.hdmf_common.VectorData( ...
38-
'description', 'random_multi column', ...
39-
'data', rand(3,2,nrows) ...
40-
);
41-
str_col = types.hdmf_common.VectorData( ...
42-
'description', 'stringdata column', ...
43-
'data', repmat({'TRUE'}, nrows, 1) ...
44-
);
45-
compound_data = types.hdmf_common.VectorData(...
46-
'description', 'compound data column', ...
47-
'data', table(rand(nrows, 1), rand(nrows, 1), 'VariableNames', {'a', 'b'}));
48-
compound_data_struct = types.hdmf_common.VectorData(...
49-
'description', 'compound data using struct', ...
50-
'data', struct('a', rand(nrows, 1), 'b', rand(nrows, 1)));
51-
file.intervals_trials = types.core.TimeIntervals(...
52-
'description', 'test dynamic table column',...
53-
'colnames', colnames, ...
54-
'start_time', start_col, ...
55-
'stop_time', stop_col, ...
56-
'randomvalues', rv_col, ...
57-
'randomvalues_index', rv_index_col, ...
58-
'randomvalues_index_index', rv_index_index_col, ...
59-
'random_multi', multi_col, ...
60-
'stringdata', str_col, ...
61-
'compound_data', compound_data, ...
62-
'compound_data_struct', compound_data_struct, ...
63-
'id', types.hdmf_common.ElementIdentifiers('data', ids(1:nrows)) ...
64-
);
65-
% check table configuration.
66-
% This can be removed when addRow is added back in
67-
types.util.dynamictable.checkConfig(file.intervals_trials)
68-
end
6913

70-
function addExpandableDynamicTable(~, file, start_array, stop_array, ...
71-
random_array, random_multi_array, ragged_data_array, ...
72-
ragged_index_array, id_array)
73-
% create VectorData objects with DataPipe objects
74-
start_time_exp = types.hdmf_common.VectorData( ...
75-
'description', 'start times', ...
76-
'data', types.untyped.DataPipe( ...
77-
'data', start_array, ...
78-
'maxSize', Inf ...
79-
) ...
80-
);
81-
stop_time_exp = types.hdmf_common.VectorData( ...
82-
'description', 'stop times', ...
83-
'data', types.untyped.DataPipe( ...
84-
'data', stop_array, ...
85-
'maxSize', Inf ...
86-
) ...
87-
);
88-
random_exp = types.hdmf_common.VectorData( ...
89-
'description', 'random data column', ...
90-
'data', types.untyped.DataPipe( ...
91-
'data', random_array', ...
92-
'maxSize', [1, Inf], ...
93-
'axis', 2 ...
94-
)...
95-
);
96-
random_multi_exp = types.hdmf_common.VectorData( ...
97-
'description', 'random data column', ...
98-
'data', types.untyped.DataPipe( ...
99-
'data', random_multi_array, ...
100-
'maxSize', [3 , 2, Inf], ...
101-
'axis', 3 ...
102-
)...
103-
);
104-
ragged_exp = types.hdmf_common.VectorData( ...
105-
'description', 'random data column', ...
106-
'data', types.untyped.DataPipe( ...
107-
'data', ragged_data_array, ...
108-
'maxSize', [Inf, 1], ...
109-
'axis', 1 ...
110-
)...
111-
);
112-
ragged_exp_index = types.hdmf_common.VectorIndex( ...
113-
'description', 'random data column', ...
114-
'target',types.untyped.ObjectView(ragged_exp), ...
115-
'data', types.untyped.DataPipe( ...
116-
'data', ragged_index_array', ...
117-
'maxSize', Inf ...
118-
)...
119-
);
120-
ids_exp = types.hdmf_common.ElementIdentifiers( ...
121-
'data', types.untyped.DataPipe( ...
122-
'data', id_array, ...
123-
'maxSize', Inf ...
124-
) ...
125-
);
126-
% create expandable table
127-
colnames = { ...
128-
'start_time', 'stop_time', ...
129-
'randomvalues', 'random_multi' ,'random_ragged' ...
130-
};
131-
file.intervals_trials = types.core.TimeIntervals( ...
132-
'description', 'test expdandable dynamic table', ...
14+
random_multi_vec = types.hdmf_common.VectorData(...
15+
'description', 'Test multi-dimensional appending using data pipes.', ...
16+
'data', types.untyped.DataPipe(...
17+
'axis', 1, ...
18+
'maxSize', [inf 3 2], ...
19+
'dataType', 'double'));
20+
file.intervals_trials = types.core.TimeIntervals(...
21+
'description', 'test dynamic table column', ...
13322
'colnames', colnames, ...
134-
'start_time', start_time_exp, ...
135-
'stop_time', stop_time_exp, ...
136-
'randomvalues', random_exp, ...
137-
'random_multi', random_multi_exp, ...
138-
'random_ragged', ragged_exp, ...
139-
'random_ragged_index', ragged_exp_index, ...
140-
'id', ids_exp ...
141-
);
142-
% check table configuration.
143-
% This can be removed when addRow is added back in
144-
types.util.dynamictable.checkConfig(file.intervals_trials)
23+
'random_multi', random_multi_vec);
24+
for iRow = 1:nrows
25+
file.intervals_trials.addRow(...
26+
'start_time', iRow, ...
27+
'stop_time', iRow, ...
28+
'randomvalues', {rand(2,3),rand(2,5)}, ...
29+
'random_multi', rand(1,3,2), ...
30+
'stringdata', {'TRUE'}, ...
31+
'id', ids(iRow), ...
32+
'compound_table', table(rand(1), rand(1), 'VariableNames', {'a', 'b'}), ...
33+
'compound_struct', struct('a', rand(), 'b', rand()), ...
34+
'logical_data', true);
35+
end
14536
end
14637

14738
function addContainerUnevenColumns(~, file)
@@ -164,6 +55,7 @@ function addContainerUnevenColumns(~, file)
16455
) ...
16556
);
16657
end
58+
16759
function addContainerUnmatchedIDs(~, file)
16860
% create and add a container with columns of unmatched length
16961
colnames = {'start_time', 'stop_time', 'randomvalues'};
@@ -187,6 +79,7 @@ function addContainerUnmatchedIDs(~, file)
18779
) ...
18880
);
18981
end
82+
19083
function addContainerUndefinedIDs(~, file)
19184
% create and add a container with undefined id field
19285
colnames = {'start_time', 'stop_time', 'randomvalues'};
@@ -207,6 +100,7 @@ function addContainerUndefinedIDs(~, file)
207100
) ...
208101
);
209102
end
103+
210104
function c = getContainer(~, file)
211105
c = file.intervals_trials.vectordata.get('randomvalues');
212106
end
@@ -231,6 +125,7 @@ function appendContainer(testCase, file)
231125
"NWB:DynamicTable" ...
232126
);
233127
end
128+
234129
function appendRaggedContainer(~, file)
235130
% create synthetic data
236131
data = (100:-1:1);
@@ -272,7 +167,7 @@ function getRowTest(testCase)
272167
else
273168
startInd = VectorDataInd.data(Indices(iRaggedInd) - 1) + 1;
274169
end
275-
dataIndices{iRaggedInd} = BaseVectorData.data((startInd:endInd) .', :);
170+
dataIndices{iRaggedInd} = BaseVectorData.data(:, (startInd:endInd) .');
276171
end
277172

278173
actualData = Table.getRow(5, 'columns', {'randomvalues'});
@@ -305,7 +200,7 @@ function getRowRoundtripTest(testCase)
305200

306201
% even if struct is passed in. It is still read back as a
307202
% table. So we cheat a bit here since this is expected a2a.
308-
CompoundStructVector = ExpectedTable.vectordata.get('compound_data_struct');
203+
CompoundStructVector = ExpectedTable.vectordata.get('compound_struct');
309204
ExpectedCompoundStruct = CompoundStructVector.data;
310205
CompoundStructVector.data = struct2table(ExpectedCompoundStruct);
311206

@@ -315,35 +210,6 @@ function getRowRoundtripTest(testCase)
315210
ActualTable.getRow([13, 19], 'useId', true));
316211
end
317212

318-
function ExpandableTableTest(testCase)
319-
% define data matrices
320-
nrows = 20;
321-
id = 0:nrows-1; % different from row poistion
322-
start_time_array = 1:nrows;
323-
stop_time_array = start_time_array + 1;
324-
rng(1); % to be able replicate random values
325-
random_val_array = rand(nrows, 1);
326-
random_multi_array = rand(3, 2, nrows);
327-
% create expandable table with first half of arrays
328-
testCase.addExpandableDynamicTable(testCase.file, ...
329-
start_time_array(1:10), stop_time_array(1:10), ...
330-
random_val_array(1:10), random_multi_array(:, : ,1:10), ...
331-
rand(nrows*3,1), [sort(randi(nrows*3,9,1)); nrows*3], id(1:10));
332-
% export and read-in expandable table
333-
filename = ['MatNWB.' testCase.className() '.ExpandableTableTest.nwb'];
334-
nwbExport(testCase.file, filename);
335-
% removing addRows test until function is updated
336-
% read in expanded table
337-
readFile = nwbRead(filename, 'ignorecache');
338-
% test getRow
339-
actualData = readFile.intervals_trials.getRow(1:10, ...
340-
'columns', {'randomvalues'});
341-
testCase.verifyEqual( ...
342-
random_val_array(1:10), ...
343-
actualData.randomvalues ...
344-
);
345-
end
346-
347213
function toTableTest(testCase)
348214
% test DynamicTable toTable method.
349215
% 1. For a generic table, the toTable output should be very
@@ -395,20 +261,21 @@ function toTableTest(testCase)
395261
);
396262
end
397263
end
264+
398265
function DynamicTableCheckTest(testCase)
399266
% Verify that the checkConfig utility function
400267
% throws error when defining an invalid table
401268
%
402269
% 1. Defining a table with columns of unmatched length
403270
testCase.verifyError( ...
404271
@() testCase.addContainerUnevenColumns(testCase.file), ...
405-
'MatNWB:DynamicTable:CheckConfig:InvalidShape' ...
272+
'NWB:DynamicTable:CheckConfig:InvalidShape' ...
406273
)
407274
% 2. Defining a table with length of id's does not match
408275
% the number of columns
409276
testCase.verifyError( ...
410277
@() testCase.addContainerUnmatchedIDs(testCase.file), ...
411-
'MatNWB:DynamicTable:CheckConfig:InvalidId' ...
278+
'NWB:DynamicTable:CheckConfig:InvalidId' ...
412279
)
413280
%3. Defining a table with unspecified IDs
414281
testCase.addContainerUndefinedIDs(testCase.file)
@@ -436,29 +303,15 @@ function testEmptyTable(testCase)
436303
'description', 'start time column', ...
437304
'data', (1:5)' ...
438305
)), ...
439-
'MatNWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
306+
'NWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
440307

441308
% validate that "vectordata" set checking works.
442309
testCase.verifyError(@()types.core.TimeIntervals(...
443310
'description', 'test error', ...
444311
'randomvalues', types.hdmf_common.VectorData( ...
445312
'description', 'random values', ...
446313
'data', mat2cell(rand(25,2), repmat(5, 5, 1)))), ...
447-
'MatNWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
314+
'NWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
448315
end
449316
end
450317
end
451-
function zipped = zipArrays(A,B)
452-
zipped = zeros((length(A)+length(B)),1);
453-
countA = 1;
454-
countB = 1;
455-
for i = 1:length(A)+length(B)
456-
if mod(i,2)
457-
zipped(i) = A(countA);
458-
countA = countA+1;
459-
else
460-
zipped(i) = B(countB);
461-
countB = countB+1;
462-
end
463-
end
464-
end

‎+tests/+unit/tutorialTest.m‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ function testTutorials(testCase)
2626
if listing.isdir || any(strcmp(skippedTutorials, listing.name))
2727
continue;
2828
end
29-
run(listing.name);
29+
try
30+
run(listing.name);
31+
catch ME
32+
error('NWB:Test:Tutorial', ...
33+
'Error while running test `%s`. Full error message:\n\n%s', listing.name, getReport(ME));
34+
end
3035
end
3136
end

‎+tests/+util/verifyContainerEqual.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ function verifyContainerEqual(testCase, actual, expected, ignoreList)
33
ignoreList = {};
44
end
55
assert(iscellstr(ignoreList),...
6-
'MatNWB:Test:InvalidIgnoreList',...
6+
'NWB:Test:InvalidIgnoreList',...
77
['Ignore List must be a cell array of character arrays indicating props that should be '...
88
'ignored.']);
99
testCase.verifyEqual(class(actual), class(expected));

0 commit comments

Comments
 (0)