Skip to content

Commit cef5ce5

Browse files
ehennestadehennestad
authored
Update fillValidators.m (#714)
* Add function for updating dataset specification from parent (unfinished) * Update fillValidators.m Dimension validation should not depend on datatype * Improve comment * Unrelated: Fix value of spike_times description in test to match pynwb * Remove debug code, add comment * Removed duplicate comment * Update Dataset.m Ensure value takes precedence over quantity when determining the required flag for a Dataset. * Move and refactor updateDatasetFromParent - Moved to schemes.internal namespace - Renamed to updateDatasetSpecFromParent - Added general utility function for matching spec with parent spec for base schema types - Added empty functions for updating groups and attributes specs from parent * Update processClass.m * Update processClass.m Fixed function name * Update processClass.m Fixed renamed variables * Update processClass.m Fixed logic for when to do an update * Update updateDatasetSpecFromParent.m Fixed logic for when to update attributes * Update updateDatasetSpecFromParent.m Fixed previous commit * Update generated types * Generate types Fixed issue where shape validation was skipped when dtype='any' * Fix test using 5D timeseries data Previous commit added shape validation of Timeseries.data, constraining it to be 4D * Update findMatchingParentSpec.m Fixed bug in matching logic. Don't want to match on neurodata_type_inc if spec has a name * Ad hoc logic for skipping test for older schema versions Add todo * Fix order of using value and default_value from source spec * Create Primitives.m * Simplify functions for resolving inherited specification fields * Renamed new functions * Rename parent to ancestor * Use consistent names across functions * Minor changes for improved clarity
1 parent 26d0dd0 commit cef5ce5

18 files changed

+168
-34
lines changed

‎+file/Attribute.m

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,16 @@
3939
obj.required = source(requiredKey);
4040
end
4141

42+
% Use either 'value' or 'default_value' (not both).
43+
% If both are present, 'value' takes precedence.
4244
valueKey = 'value';
4345
defaultKey = 'default_value';
44-
if isKey(source, defaultKey)
45-
obj.value = source(defaultKey);
46-
obj.readonly = false;
47-
elseif isKey(source, valueKey)
46+
if isKey(source, valueKey)
4847
obj.value = source(valueKey);
4948
obj.readonly = true;
49+
elseif isKey(source, defaultKey)
50+
obj.value = source(defaultKey);
51+
obj.readonly = false;
5052
else
5153
obj.value = [];
5254
obj.readonly = false;

‎+file/Dataset.m

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,16 @@
4949
end
5050

5151
% Todo: same as for attribute, should consolidate
52+
% Use either 'value' or 'default_value' (not both).
53+
% If both are present, 'value' takes precedence.
5254
valueKey = 'value';
5355
defaultKey = 'default_value';
54-
if isKey(source, defaultKey)
55-
obj.value = source(defaultKey);
56-
obj.readonly = false;
57-
elseif isKey(source, valueKey)
56+
if isKey(source, valueKey)
5857
obj.value = source(valueKey);
5958
obj.readonly = true;
59+
elseif isKey(source, defaultKey)
60+
obj.value = source(defaultKey);
61+
obj.readonly = false;
6062
else
6163
obj.value = [];
6264
obj.readonly = false;
@@ -78,7 +80,11 @@
7880
obj.dtype = file.mapType(source(dataTypeKey));
7981
end
8082

81-
if isKey(source, 'quantity')
83+
% If a value key is specified, the resulting property is a
84+
% constant (and by definition required). Therefore we will only
85+
% update the required flag based on the `quantity` key when the
86+
% `value` key is missing.
87+
if ~isKey(source, valueKey) && isKey(source, 'quantity')
8288
obj.required = obj.isRequired(source);
8389
obj.scalar = obj.isScalar(source);
8490
end

‎+file/fillValidators.m

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
if (isa(prop, 'file.Attribute') || isa(prop, 'file.Dataset')) ...
88
&& prop.readonly && ~isempty(prop.value)
99
% Need to add a validator for inherited and readonly properties. In
10-
% the superclass these properties might not be read only and due to
11-
% inheritance its not possible to change property attributes
10+
% the superclass these properties might not be read-only and due to
11+
% inheritance rules in MATLAB it is not possible to change property
12+
% attributes of a property from public (in a superclass) to
13+
% protected (in a subclass).
1214
if any(strcmp(nm, inherited))
1315
validationBody = fillReadOnlyValidator(nm, prop.value, className);
1416
else
@@ -57,7 +59,7 @@
5759
elseif isa(prop, 'file.Attribute')
5860
unitValidationStr = strjoin({unitValidationStr...
5961
fillDtypeValidation(name, prop.dtype)...
60-
fillDimensionValidation(name, prop.dtype, prop.shape)...
62+
fillDimensionValidation(name, prop.shape)...
6163
}, newline);
6264
else % Link
6365
fullname = namespaceReg.getFullClassName(prop.type);
@@ -154,7 +156,7 @@
154156
if isempty(prop.type)
155157
unitValidationStr = strjoin({unitValidationStr...
156158
fillDtypeValidation(name, prop.dtype)...
157-
fillDimensionValidation(name, prop.dtype, prop.shape)...
159+
fillDimensionValidation(name, prop.shape)...
158160
}, newline);
159161
elseif prop.isConstrainedSet
160162
fullname = getFullClassName(namespaceReg, prop.type, name);
@@ -200,11 +202,7 @@
200202
);
201203
end
202204

203-
function fdvstr = fillDimensionValidation(name, type, shape)
204-
if strcmp(type, 'any')
205-
fdvstr = '';
206-
return;
207-
end
205+
function fdvstr = fillDimensionValidation(name, shape)
208206

209207
if iscell(shape)
210208
if ~isempty(shape) && iscell(shape{1})

‎+file/processClass.m

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
branch = [{namespace.getClass(name)} namespace.getRootBranch(name)];
44
branchNames = cell(size(branch));
55
TYPEDEF_KEYS = {'neurodata_type_def', 'data_type_def'};
6+
67
for i = 1:length(branch)
78
hasTypeDefs = isKey(branch{i}, TYPEDEF_KEYS);
89
branchNames{i} = branch{i}(TYPEDEF_KEYS{hasTypeDefs});
@@ -14,6 +15,9 @@
1415
nodename = node(TYPEDEF_KEYS{hasTypeDefs});
1516

1617
if ~isKey(pregen, nodename)
18+
19+
spec.internal.resolveInheritedFields(node, branch(iAncestor+1:end))
20+
1721
switch node('class_type')
1822
case 'groups'
1923
class = file.Group(node);

‎+spec/+enum/Primitives.m

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
classdef Primitives
2+
enumeration
3+
Groups('groups')
4+
Datasets('datasets')
5+
Attributes('attributes')
6+
Links('links')
7+
end
8+
9+
properties
10+
Key
11+
end
12+
13+
methods
14+
function obj = Primitives(keyName)
15+
obj.Key = keyName;
16+
end
17+
end
18+
end

‎+spec/+internal/findMatchingAncestorSpec.m

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
function matchingAncestorSpec = findMatchingAncestorSpec(spec, ancestorSpecs)
2+
% findMatchingAncestorSpec - Find a spec from a set of ancestor specs matching on name
3+
4+
matchingAncestorSpec = [];
5+
6+
if isKey(spec, 'name')
7+
specId = spec('name');
8+
9+
for j = 1:numel(ancestorSpecs)
10+
ancestorSpec = ancestorSpecs{j};
11+
12+
if isKey(ancestorSpec, 'name') && strcmp(ancestorSpec('name'), specId)
13+
matchingAncestorSpec = ancestorSpec;
14+
break
15+
end
16+
end
17+
end
18+
end

‎+spec/+internal/resolveInheritedFields.m

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
function resolveInheritedFields(typeSpec, ancestorTypeSpecs)
2+
% resolveInheritedFields - Resolve inherited fields from ancestor specification types.
3+
4+
% Update type specifications based on ancestor types. In the schema
5+
% specification, types implicitly inherit keys from the corresponding
6+
% group, dataset, attribute, or link definitions of their ancestor types.
7+
% If a key is redefined in a subtype, only the overridden keys are typically
8+
% specified. To ensure that downstream generator classes use the inherited
9+
% specification values (instead of default ones), we loop through the type
10+
% hierarchy and fill in any missing key/value pairs from the ancestor
11+
% specifications.
12+
13+
primitiveTypes = enumeration('spec.enum.Primitives');
14+
15+
for i = 1:length(ancestorTypeSpecs)
16+
ancestorType = ancestorTypeSpecs{i};
17+
18+
for j = 1:numel(primitiveTypes)
19+
primitiveKey = primitiveTypes(j).Key; % i.e: 'groups', 'datasets' etc.
20+
if isKey(typeSpec, primitiveKey) && isKey(ancestorType, primitiveKey)
21+
spec.internal.updateSpecFromAncestorSpec(...
22+
typeSpec(primitiveKey), ancestorType(primitiveKey))
23+
end
24+
end
25+
end
26+
end

‎+spec/+internal/updateSpecFromAncestorSpec.m

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
function updateSpecFromAncestorSpec(childSpecs, ancestorSpecs)
2+
% updateSpecFromAncestorSpec - Updates child specifications from ancestor specifications.
3+
%
4+
% Syntax:
5+
% spec.internal.updateSpecFromAncestorSpec(childSpecs, ancestorSpecs)
6+
%
7+
% Input Arguments:
8+
% childSpecs - Cell array of child specification containers.Map objects.
9+
% ancestorSpecs - Cell array of ancestor specification containers.Map objects.
10+
%
11+
% Output Arguments:
12+
% None. The function modifies child specifications in place.
13+
14+
primitiveTypes = enumeration('spec.enum.Primitives');
15+
primitiveTypeKeys = {primitiveTypes.Key};
16+
17+
for iSpec = 1:numel(childSpecs)
18+
currentSpec = childSpecs{iSpec};
19+
20+
matchingAncestorSpec = spec.internal.findMatchingAncestorSpec(...
21+
currentSpec, ancestorSpecs);
22+
23+
if isempty(matchingAncestorSpec)
24+
continue
25+
end
26+
27+
ancestorSpecKeys = matchingAncestorSpec.keys();
28+
for iKey = 1:numel(ancestorSpecKeys)
29+
currentAncestorKey = ancestorSpecKeys{iKey};
30+
if any(strcmp(currentAncestorKey, primitiveTypeKeys))
31+
if isKey(currentSpec, currentAncestorKey)
32+
% Recursively update nested specification primitives
33+
spec.internal.updateSpecFromAncestorSpec(...
34+
currentSpec(currentAncestorKey), ...
35+
matchingAncestorSpec(currentAncestorKey))
36+
end
37+
elseif ~isKey(currentSpec, currentAncestorKey)
38+
currentSpec(currentAncestorKey) = matchingAncestorSpec(currentAncestorKey);
39+
end
40+
end
41+
end
42+
end

‎+tests/+unit/+types/SpikeEventSeriesTest.m

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,22 @@ function setupTemporaryWorkingFolder(testCase)
1010
methods (Test)
1111
function testExportSpikeEventSeries(testCase)
1212

13+
% This test should only run for NWB v2.9.0 or newer
14+
versionToTest = [2, 9, 0];
15+
16+
currentVersionStr = types.core.Version();
17+
currentVersionNum = str2double( strsplit(currentVersionStr, '.') );
18+
19+
for i = 1:3
20+
if currentVersionNum(i) < versionToTest(i)
21+
return
22+
elseif currentVersionNum(i) > versionToTest(i)
23+
break
24+
elseif currentVersionNum(i) == versionToTest(i)
25+
continue
26+
end
27+
end
28+
1329
nwbFile = tests.factory.NWBFile();
1430
electrodeTable = tests.factory.ElectrodeTable(nwbFile);
1531

‎+tests/+unit/dataStubTest.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function testRegionRead(testCase)
1212

1313
nwb = tests.factory.NWBFile();
1414

15-
data = reshape(1:5000, 25, 5, 4, 2, 5);
15+
data = reshape(1:5000, 25, 5, 4, 10);
1616

1717
timeseries = types.core.TimeSeries(...
1818
'starting_time', 0.0, ... % seconds
@@ -37,7 +37,7 @@ function testRegionRead(testCase)
3737
testCase.verifyEqual(stubData, stub.load([2, 2, 2], [4, 4, 4]));
3838

3939
% test Inf
40-
testCase.verifyEqual(stub(2:end, 2:end, 2:end, :), data(2:end, 2:end, 2:end, :));
40+
testCase.verifyEqual(stub(2:end, 2:end, :), data(2:end, 2:end, :));
4141

4242
% test stride
4343
testCase.verifyEqual(stub(1:2:25, 1:2:4, :, :), data(1:2:25, 1:2:4, :, :));
@@ -56,7 +56,7 @@ function testRegionRead(testCase)
5656
testCase.verifyEqual(stub(primeInd, 2:4, :), data(primeInd, 2:4, :));
5757
testCase.verifyEqual(stub(primeInd, :, 1), data(primeInd, :, 1));
5858
testCase.verifyEqual(stub(primeInd, [1 2 5]), data(primeInd, [1 2 5]));
59-
testCase.verifyEqual(stub([1 25], [1 5], [1 4], [1 2], [1 5]), data([1 25], [1 5], [1 4], [1 2], [1 5]));
59+
testCase.verifyEqual(stub([1 25], [1 5], [1 4], [1 10]), data([1 25], [1 5], [1 4], [1 10]));
6060
overflowPrimeInd = primes(31);
6161
testCase.verifyEqual(stub(overflowPrimeInd), stub(ind2sub(stub.dims, overflowPrimeInd)));
6262
testCase.verifyEqual(stub(overflowPrimeInd), data(overflowPrimeInd));
@@ -65,7 +65,7 @@ function testRegionRead(testCase)
6565
testCase.verifyEqual(stub([1 1 1 1]), data([1 1 1 1]));
6666

6767
% test out of order indices
68-
testCase.verifyEqual(stub([5 4 3 2 2]), data([5 4 3 2 2]));
68+
testCase.verifyEqual(stub([5 4 3 10]), data([5 4 3 10]));
6969
end
7070

7171
function testObjectCopy(testCase)

‎+types/+core/CurrentClampSeries.m

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
%
3535
% - control_description (char) - Description of each control value. Must be present if control is present. If present, control_description[0] should describe time points where control == 0.
3636
%
37-
% - data (any) - Recorded voltage.
37+
% - data (numeric) - Recorded voltage.
3838
%
3939
% - data_continuity (char) - Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or "step". For example, a voltage trace would be "continuous", because samples are recorded from a continuous process. An array of lick times would be "instantaneous", because the data represents distinct moments in time. Times of image presentations would be "step" because the picture remains the same until the next timepoint. This field is optional, but is useful in providing information about the underlying data. It may inform the way this data is interpreted, the way it is visualized, and what analysis methods are applicable.
4040
%
@@ -108,7 +108,8 @@
108108
types.util.validateShape('capacitance_compensation', {[1]}, val)
109109
end
110110
function val = validate_data(obj, val)
111-
111+
val = types.util.checkDtype('data', 'numeric', val);
112+
types.util.validateShape('data', {[Inf]}, val)
112113
end
113114
function val = validate_data_unit(obj, val)
114115
if isequal(val, 'volts')

‎+types/+core/CurrentClampStimulusSeries.m

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
%
2323
% - control_description (char) - Description of each control value. Must be present if control is present. If present, control_description[0] should describe time points where control == 0.
2424
%
25-
% - data (any) - Stimulus current applied.
25+
% - data (numeric) - Stimulus current applied.
2626
%
2727
% - data_continuity (char) - Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or "step". For example, a voltage trace would be "continuous", because samples are recorded from a continuous process. An array of lick times would be "instantaneous", because the data represents distinct moments in time. Times of image presentations would be "step" because the picture remains the same until the next timepoint. This field is optional, but is useful in providing information about the underlying data. It may inform the way this data is interpreted, the way it is visualized, and what analysis methods are applicable.
2828
%
@@ -70,7 +70,8 @@
7070
%% VALIDATORS
7171

7272
function val = validate_data(obj, val)
73-
73+
val = types.util.checkDtype('data', 'numeric', val);
74+
types.util.validateShape('data', {[Inf]}, val)
7475
end
7576
function val = validate_data_unit(obj, val)
7677
if isequal(val, 'amperes')

‎+types/+core/IZeroClampSeries.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
%
2323
% - control_description (char) - Description of each control value. Must be present if control is present. If present, control_description[0] should describe time points where control == 0.
2424
%
25-
% - data (any) - Recorded voltage.
25+
% - data (numeric) - Recorded voltage.
2626
%
2727
% - data_continuity (char) - Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or "step". For example, a voltage trace would be "continuous", because samples are recorded from a continuous process. An array of lick times would be "instantaneous", because the data represents distinct moments in time. Times of image presentations would be "step" because the picture remains the same until the next timepoint. This field is optional, but is useful in providing information about the underlying data. It may inform the way this data is interpreted, the way it is visualized, and what analysis methods are applicable.
2828
%

‎+types/+core/SpikeEventSeries.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
% SPIKEEVENTSERIES - Stores snapshots/snippets of recorded spike events (i.e., threshold crossings). This may also be raw data, as reported by ephys hardware. If so, the TimeSeries::description field should describe how events were detected. All events span the same recording channels and store snapshots of equal duration. TimeSeries::data array structure: [num events] [num channels] [num samples] (or [num events] [num samples] for single electrode).
33
%
44
% Required Properties:
5-
% data, electrodes, timestamps
5+
% data, electrodes
66

77

88

‎+types/+core/TimeSeries.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ function postset_starting_time_rate(obj)
207207
types.util.validateShape('control_description', {[Inf]}, val)
208208
end
209209
function val = validate_data(obj, val)
210-
210+
types.util.validateShape('data', {[Inf,Inf,Inf,Inf], [Inf,Inf,Inf], [Inf,Inf], [Inf]}, val)
211211
end
212212
function val = validate_data_continuity(obj, val)
213213
val = types.util.checkDtype('data_continuity', 'char', val);

‎+types/+core/VoltageClampSeries.m

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
%
4747
% - control_description (char) - Description of each control value. Must be present if control is present. If present, control_description[0] should describe time points where control == 0.
4848
%
49-
% - data (any) - Recorded current.
49+
% - data (numeric) - Recorded current.
5050
%
5151
% - data_continuity (char) - Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or "step". For example, a voltage trace would be "continuous", because samples are recorded from a continuous process. An array of lick times would be "instantaneous", because the data represents distinct moments in time. Times of image presentations would be "step" because the picture remains the same until the next timepoint. This field is optional, but is useful in providing information about the underlying data. It may inform the way this data is interpreted, the way it is visualized, and what analysis methods are applicable.
5252
%
@@ -160,7 +160,8 @@
160160
types.util.validateShape('capacitance_slow', {[1]}, val)
161161
end
162162
function val = validate_data(obj, val)
163-
163+
val = types.util.checkDtype('data', 'numeric', val);
164+
types.util.validateShape('data', {[Inf]}, val)
164165
end
165166
function val = validate_data_unit(obj, val)
166167
if isequal(val, 'amperes')

‎+types/+core/VoltageClampStimulusSeries.m

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
%
2323
% - control_description (char) - Description of each control value. Must be present if control is present. If present, control_description[0] should describe time points where control == 0.
2424
%
25-
% - data (any) - Stimulus voltage applied.
25+
% - data (numeric) - Stimulus voltage applied.
2626
%
2727
% - data_continuity (char) - Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or "step". For example, a voltage trace would be "continuous", because samples are recorded from a continuous process. An array of lick times would be "instantaneous", because the data represents distinct moments in time. Times of image presentations would be "step" because the picture remains the same until the next timepoint. This field is optional, but is useful in providing information about the underlying data. It may inform the way this data is interpreted, the way it is visualized, and what analysis methods are applicable.
2828
%
@@ -70,7 +70,8 @@
7070
%% VALIDATORS
7171

7272
function val = validate_data(obj, val)
73-
73+
val = types.util.checkDtype('data', 'numeric', val);
74+
types.util.validateShape('data', {[Inf]}, val)
7475
end
7576
function val = validate_data_unit(obj, val)
7677
if isequal(val, 'volts')

‎+types/+hdmf_common/CSRMatrix.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
%% VALIDATORS
7272

7373
function val = validate_data(obj, val)
74-
74+
types.util.validateShape('data', {[Inf]}, val)
7575
end
7676
function val = validate_indices(obj, val)
7777
val = types.util.checkDtype('indices', 'uint', val);

0 commit comments

Comments
 (0)