Skip to content

Commit c22e7d6

Browse files
ehennestadehennestad
authored
Fix parsing of quantity specification value (#695)
* Create HasQuantity.m * Add HasQuantity as superclass to Group, Dataset and Link * Add specification and test to test that specification quantities are correctly parsed * Fix bug in HasQuantity * Update MetaClass.m Expose getRequiredMethods publicly (but keep hidden) * Add tests for invalid quantity specifications * Remove debug statement
1 parent 7090ed0 commit c22e7d6

File tree

8 files changed

+231
-56
lines changed

8 files changed

+231
-56
lines changed

‎+file/+interface/HasQuantity.m

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
classdef HasQuantity
2+
% HasQuantity - Provide methods for parsing quantity specification value
3+
4+
% properties
5+
% QuantityKey = 'quantity'
6+
% end
7+
8+
methods (Static, Access = protected)
9+
function isRequired = isRequired(source)
10+
if isKey(source, 'quantity')
11+
quantity = source('quantity');
12+
file.interface.HasQuantity.validateQuantity(quantity)
13+
14+
if ischar(quantity)
15+
switch quantity
16+
case {'?', 'zero_or_one'}
17+
isRequired = false;
18+
case {'*', 'zero_or_many'}
19+
isRequired = false;
20+
case {'+', 'one_or_many'}
21+
isRequired = true;
22+
end
23+
elseif isnumeric(quantity)
24+
isRequired = quantity >= 1;
25+
end
26+
else
27+
isRequired = true; % Default
28+
end
29+
end
30+
31+
function isScalar = isScalar(source)
32+
if isKey(source, 'quantity')
33+
quantity = source('quantity');
34+
file.interface.HasQuantity.validateQuantity(quantity)
35+
36+
if ischar(quantity)
37+
switch quantity
38+
case {'?', 'zero_or_one'}
39+
isScalar = true;
40+
case {'*', 'zero_or_many'}
41+
isScalar = false;
42+
case {'+', 'one_or_many'}
43+
isScalar = false;
44+
end
45+
elseif isnumeric(quantity)
46+
if quantity == 1
47+
isScalar = true;
48+
else
49+
isScalar = false;
50+
end
51+
end
52+
else
53+
isScalar = true; % Default
54+
end
55+
end
56+
end
57+
58+
methods (Static, Access = private)
59+
function validateQuantity(quantity)
60+
% validateQuantity - Validate quantity specification value
61+
if ischar(quantity)
62+
validQuantities = [ ...
63+
"?", "zero_or_one", ...
64+
"*", "zero_or_many", ...
65+
"+", "one_or_many" ...
66+
];
67+
if ~any(strcmp(validQuantities, quantity))
68+
validQuantitiesStr = strjoin(" " + validQuantities, newline);
69+
ME = MException('NWB:Schema:UnsupportedQuantity', ...
70+
['Quantity is "%s", but expected quantity to be one of the ' ...
71+
'following values:\n%s\n'], quantity, validQuantitiesStr);
72+
throwAsCaller(ME)
73+
end
74+
75+
elseif isnumeric(quantity)
76+
assert( mod(quantity,1) == 0 && quantity > 0, ...
77+
'NWB:Schema:UnsupportedQuantity', ...
78+
'Expected quantity to positive integer')
79+
else
80+
ME = MException('NWB:Schema:UnsupportedQuantity', ...
81+
'Expected quantity to be text or numeric.');
82+
throwAsCaller(ME)
83+
end
84+
end
85+
end
86+
end

‎+file/Dataset.m

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Dataset < file.interface.HasProps
1+
classdef Dataset < file.interface.HasProps & file.interface.HasQuantity
22
properties
33
name;
44
doc;
@@ -79,18 +79,8 @@
7979
end
8080

8181
if isKey(source, 'quantity')
82-
quantity = source('quantity');
83-
switch quantity
84-
case '?'
85-
obj.required = false;
86-
obj.scalar = true;
87-
case '*'
88-
obj.required = false;
89-
obj.scalar = false;
90-
case '+'
91-
obj.required = true;
92-
obj.scalar = false;
93-
end
82+
obj.required = obj.isRequired(source);
83+
obj.scalar = obj.isScalar(source);
9484
end
9585

9686
obj.isConstrainedSet = ~isempty(obj.type) && ~obj.scalar;

‎+file/Group.m

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Group < file.interface.HasProps
1+
classdef Group < file.interface.HasProps & file.interface.HasQuantity
22
properties
33
doc;
44
name;
@@ -65,18 +65,8 @@
6565
end
6666

6767
if isKey(source, 'quantity')
68-
quantity = source('quantity');
69-
switch quantity
70-
case '?'
71-
obj.required = false;
72-
obj.scalar = true;
73-
case '*'
74-
obj.required = false;
75-
obj.scalar = false;
76-
case '+'
77-
obj.required = true;
78-
obj.scalar = false;
79-
end
68+
obj.required = obj.isRequired(source);
69+
obj.scalar = obj.isScalar(source);
8070
end
8171

8272
obj.isConstrainedSet = ~obj.scalar && ~isempty(obj.type);
@@ -144,7 +134,7 @@
144134
PropertyMap = containers.Map;
145135
%typed + constrained
146136
%should never happen
147-
137+
148138
if obj.isConstrainedSet && ~obj.definesType
149139
error('NWB:Group:UnsupportedOperation', ...
150140
'The method `getProps` should not be called on a constrained dataset.');
@@ -260,7 +250,6 @@
260250
end
261251
end
262252
end
263-
264253
end
265254
end
266255
end

‎+file/Link.m

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Link
1+
classdef Link < file.interface.HasQuantity
22
properties (SetAccess = private)
33
doc;
44
name;
@@ -19,12 +19,7 @@
1919
obj.doc = source('doc');
2020
obj.name = source('name');
2121
obj.type = source('target_type');
22-
23-
quantityKey = 'quantity';
24-
if isKey(source, quantityKey)
25-
quantity = source(quantityKey);
26-
obj.required = strcmp(quantity, '+');
27-
end
22+
obj.required = obj.isRequired(source);
2823
end
2924
end
3025
end

‎+tests/+unit/+schema/QuantityTest.m

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
classdef QuantityTest < tests.unit.abstract.SchemaTest
2+
3+
properties (Constant)
4+
SchemaFolder = "quantitySchema"
5+
SchemaNamespaceFileName = "quantity.namespace.yaml"
6+
end
7+
8+
methods (Test)
9+
function testValidQuantitySpecifications(testCase)
10+
quantContainer = types.quantity.QuantityContainer();
11+
12+
expectedRequiredProperties = [ ...
13+
"data_required_array", ...
14+
"data_required_array_long_form", ...
15+
"data_required_array_short_form", ...
16+
"data_required_scalar" ...
17+
];
18+
expectedOptionalProperties = [ ...
19+
"data_optional_array_long_form", ...
20+
"data_optional_array_short_form", ...
21+
"data_optional_scalar_long_form", ...
22+
"data_optional_scalar_short_form"
23+
];
24+
25+
actualRequiredProperties = string(quantContainer.getRequiredProperties());
26+
testCase.verifyEqual(actualRequiredProperties, expectedRequiredProperties)
27+
28+
allProperties = properties(quantContainer)';
29+
actualOptionalProperties = string(setdiff(allProperties, actualRequiredProperties));
30+
testCase.verifyEqual(actualOptionalProperties, expectedOptionalProperties)
31+
end
32+
33+
function testInvalidTextQuantitySpecification(testCase)
34+
% Simulate a dataset specification with an invalid quantity value
35+
specMap = containers.Map('quantity', 'none');
36+
testCase.verifyError(...
37+
@() file.Dataset(specMap), ...
38+
'NWB:Schema:UnsupportedQuantity')
39+
end
40+
41+
function testInvalidNumericQuantitySpecification(testCase)
42+
% Simulate a dataset specification with an invalid numeric quantity value
43+
specMap = containers.Map('quantity', 1.5);
44+
45+
testCase.verifyError(...
46+
@() file.Dataset(specMap), ...
47+
'NWB:Schema:UnsupportedQuantity')
48+
end
49+
50+
function testInvalidArrayQuantitySpecification(testCase)
51+
% Simulate a dataset specification with an invalid numeric quantity value
52+
specMap = containers.Map('quantity', {{'*', '+'}});
53+
54+
testCase.verifyError(...
55+
@() file.Dataset(specMap), ...
56+
'NWB:Schema:UnsupportedQuantity')
57+
end
58+
end
59+
end

‎+tests/test-schema/quantitySchema/quantity.namespace.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
namespaces:
2+
- full_name: Quantity specification values test
3+
name: quantity
4+
schema:
5+
- namespace: core
6+
- source: quantity.quantities.yaml
7+
version: 1.0.0

‎+tests/test-schema/quantitySchema/quantity.quantities.yaml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
groups:
2+
- neurodata_type_def: QuantityContainer
3+
neurodata_type_inc: NWBDataInterface
4+
datasets:
5+
- name: data_optional_scalar_short_form
6+
dtype: double
7+
quantity: '?'
8+
shape:
9+
- null
10+
- name: data_optional_scalar_long_form
11+
dtype: double
12+
quantity: 'zero_or_one'
13+
shape:
14+
- null
15+
- name: data_optional_array_short_form
16+
dtype: double
17+
quantity: '*'
18+
shape:
19+
- null
20+
- name: data_optional_array_long_form
21+
dtype: double
22+
quantity: 'zero_or_many'
23+
shape:
24+
- null
25+
- name: data_required_array_short_form
26+
dtype: double
27+
quantity: '+'
28+
shape:
29+
- null
30+
- name: data_required_array_long_form
31+
dtype: double
32+
quantity: 'one_or_many'
33+
shape:
34+
- null
35+
- name: data_required_scalar
36+
dtype: double
37+
quantity: 1
38+
shape:
39+
- null
40+
- name: data_required_array
41+
dtype: double
42+
quantity: 2
43+
shape:
44+
- null

‎+types/+untyped/MetaClass.m

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,32 @@ function warnIfPropertyAttributeNotExported(obj, propName, depPropName, fullpath
140140
warning(warningId, warningMessage) %#ok<SPWRN>
141141
end
142142
end
143+
144+
methods (Hidden)
145+
% Set of methods that should be publicly available, for example for
146+
% testing purposes, or other use cases where type inspection might
147+
% be necessary.
148+
function requiredProps = getRequiredProperties(obj)
149+
150+
% Introspectively retrieve required properties and add to
151+
% persistent cache/map.
152+
153+
typeClassName = class(obj);
154+
typeNamespaceVersion = getNamespaceVersionForType(typeClassName);
155+
156+
typeKey = sprintf('%s_%s', typeClassName, typeNamespaceVersion);
157+
158+
if isKey(obj.REQUIRED, typeKey)
159+
requiredProps = obj.REQUIRED( typeKey );
160+
else
161+
mc = metaclass(obj);
162+
propertyDescription = {mc.PropertyList.Description};
163+
isRequired = startsWith(propertyDescription, 'REQUIRED');
164+
requiredProps = {mc.PropertyList(isRequired).Name};
165+
obj.REQUIRED( typeKey ) = requiredProps;
166+
end
167+
end
168+
end
143169

144170
methods (Access = protected) % Override matlab.mixin.CustomDisplay
145171
function str = getFooter(obj)
@@ -229,27 +255,6 @@ function checkCustomConstraint(obj) %#ok<MANU>
229255
end
230256

231257
methods (Access = private)
232-
function requiredProps = getRequiredProperties(obj)
233-
234-
% Introspectively retrieve required properties and add to
235-
% persistent cache/map.
236-
237-
typeClassName = class(obj);
238-
typeNamespaceVersion = getNamespaceVersionForType(typeClassName);
239-
240-
typeKey = sprintf('%s_%s', typeClassName, typeNamespaceVersion);
241-
242-
if isKey(obj.REQUIRED, typeKey)
243-
requiredProps = obj.REQUIRED( typeKey );
244-
else
245-
mc = metaclass(obj);
246-
propertyDescription = {mc.PropertyList.Description};
247-
isRequired = startsWith(propertyDescription, 'REQUIRED');
248-
requiredProps = {mc.PropertyList(isRequired).Name};
249-
obj.REQUIRED( typeKey ) = requiredProps;
250-
end
251-
end
252-
253258
function tf = propertyValueEqualsDefaultValue(obj, propName)
254259
% propertyValueEqualsDefaultValue - Check if value of property is
255260
% equal to the property's default value

0 commit comments

Comments
 (0)