Skip to content

Commit 16a5411

Browse files
lawrence-mbflawrence-mbf
authored
Fix Default Subclass Properties in checkUnset (#475)
* auto-indent fillProps * auto-indent fillConstructor * make file objects non-handle * fix constrained processing for constructors Remove dependency on defined class names and instead operate on all properties. * Support Set appending when parsing constrained Originally was all-or-nothing. Now detects if a Set already exists and appends instead when that occurs. --------- Co-authored-by: Lawrence Niu <[email protected]>
1 parent 9400310 commit 16a5411

File tree

8 files changed

+284
-254
lines changed

8 files changed

+284
-254
lines changed

‎+file/Attribute.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Attribute < handle
1+
classdef Attribute
22
properties
33
name; %attribute key
44
doc; %doc string

‎+file/Dataset.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Dataset < handle
1+
classdef Dataset
22
properties
33
name;
44
doc;

‎+file/Group.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
classdef Group < handle
1+
classdef Group
22
properties
33
doc;
44
name;

‎+file/Link.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
classdef Link < handle
2-
properties(SetAccess=private)
1+
classdef Link
2+
properties (SetAccess = private)
33
doc;
44
name;
55
required;

‎+file/fillClass.m‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@
8787
name,...
8888
depnm,...
8989
defaults,... %all defaults, regardless of inheritance
90-
[required optional],...
9190
classprops,...
9291
namespace);
9392
setterFcns = file.fillSetters(setdiff(nonInherited, readonly));

‎+file/fillConstructor.m‎

Lines changed: 153 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -1,175 +1,176 @@
1-
function fcstr = fillConstructor(name, parentname, defaults, propnames, props, namespace)
2-
caps = upper(name);
3-
fcnbody = ['% ' caps ' Constructor for ' name];
1+
function fcstr = fillConstructor(name, parentname, defaults, props, namespace)
2+
caps = upper(name);
3+
fcnbody = ['% ' caps ' Constructor for ' name];
44

5-
txt = fillBody(parentname, defaults, propnames, props, namespace);
6-
if ~isempty(txt)
7-
fcnbody = [fcnbody newline txt];
8-
end
5+
txt = fillBody(parentname, defaults, props, namespace);
6+
if ~isempty(txt)
7+
fcnbody = [fcnbody newline txt];
8+
end
99

10-
fcnbody = strjoin({fcnbody,...
11-
sprintf('if strcmp(class(obj), ''%s'')', namespace.getFullClassName(name)),...
12-
' types.util.checkUnset(obj, unique(varargin(1:2:end)));',...
13-
'end'}, newline);
10+
fcnbody = strjoin({fcnbody,...
11+
sprintf('if strcmp(class(obj), ''%s'')', namespace.getFullClassName(name)),...
12+
' types.util.checkUnset(obj, unique(varargin(1:2:end)));',...
13+
'end'}, newline);
1414

15-
% insert check for DynamicTable class and child classes
16-
txt = fillCheck(name, namespace);
17-
if ~isempty(txt)
18-
fcnbody = [fcnbody newline txt];
19-
end
15+
% insert check for DynamicTable class and child classes
16+
txt = fillCheck(name, namespace);
17+
if ~isempty(txt)
18+
fcnbody = [fcnbody newline txt];
19+
end
2020

21-
fcstr = strjoin({...
22-
['function obj = ' name '(varargin)']...
23-
file.addSpaces(fcnbody, 4)...
24-
'end'}, newline);
21+
fcstr = strjoin({...
22+
['function obj = ' name '(varargin)']...
23+
file.addSpaces(fcnbody, 4)...
24+
'end'}, newline);
2525

2626
end
2727

28-
function bodystr = fillBody(pname, defaults, names, props, namespace)
29-
if isempty(defaults)
30-
bodystr = '';
31-
else
32-
overridemap = containers.Map;
33-
for i=1:length(defaults)
34-
nm = defaults{i};
35-
if strcmp(props(nm).dtype, 'char')
36-
overridemap(nm) = ['''' props(nm).value ''''];
37-
else
38-
overridemap(nm) =...
39-
sprintf('types.util.correctType(%d, ''%s'')',...
40-
props(nm).value,...
41-
props(nm).dtype);
28+
function bodystr = fillBody(pname, defaults, props, namespace)
29+
if isempty(defaults)
30+
bodystr = '';
31+
else
32+
overridemap = containers.Map;
33+
for i=1:length(defaults)
34+
nm = defaults{i};
35+
if strcmp(props(nm).dtype, 'char')
36+
overridemap(nm) = ['''' props(nm).value ''''];
37+
else
38+
overridemap(nm) =...
39+
sprintf('types.util.correctType(%d, ''%s'')',...
40+
props(nm).value,...
41+
props(nm).dtype);
42+
end
4243
end
44+
kwargs = io.map2kwargs(overridemap);
45+
%add surrounding quotes to kwargs so misc.cellPrettyPrint can print them correctly
46+
kwargs(1:2:end) = strcat('''', kwargs(1:2:end), '''');
47+
bodystr = ['varargin = [{' misc.cellPrettyPrint(kwargs) '} varargin];' newline];
4348
end
44-
kwargs = io.map2kwargs(overridemap);
45-
%add surrounding quotes to kwargs so misc.cellPrettyPrint can print them correctly
46-
kwargs(1:2:end) = strcat('''', kwargs(1:2:end), '''');
47-
bodystr = ['varargin = [{' misc.cellPrettyPrint(kwargs) '} varargin];' newline];
48-
end
49-
bodystr = [bodystr 'obj = obj@' pname '(varargin{:});'];
49+
bodystr = [bodystr 'obj = obj@' pname '(varargin{:});'];
5050

51-
if isempty(names)
52-
return;
53-
end
54-
% if there's a root object that is a constrained set, let it be hoistable from dynamic arguments
55-
dynamicConstrained = false(size(names));
56-
anon = false(size(names));
57-
isattr = false(size(names));
58-
typenames = repmat({''}, size(names));
59-
varnames = repmat({''}, size(names));
60-
for i=1:length(names)
61-
nm = names{i};
62-
prop = props(nm);
63-
64-
if isa(prop, 'file.Group') || isa(prop, 'file.Dataset')
65-
dynamicConstrained(i) = prop.isConstrainedSet && strcmpi(nm, prop.type);
66-
anon(i) = ~prop.isConstrainedSet && isempty(prop.name);
67-
68-
if ~isempty(prop.type)
69-
varnames{i} = nm;
70-
try
71-
typenames{i} = namespace.getFullClassName(prop.type);
72-
catch ME
73-
if ~strcmp(ME.identifier, 'NWB:Scheme:Namespace:NotFound')
74-
rethrow(ME);
51+
names = keys(props);
52+
if isempty(names)
53+
return;
54+
end
55+
% if there's a root object that is a constrained set, let it be hoistable from dynamic arguments
56+
dynamicConstrained = false(size(names));
57+
anon = false(size(names));
58+
isattr = false(size(names));
59+
typenames = repmat({''}, size(names));
60+
varnames = repmat({''}, size(names));
61+
for i=1:length(names)
62+
nm = names{i};
63+
prop = props(nm);
64+
65+
if isa(prop, 'file.Group') || isa(prop, 'file.Dataset')
66+
dynamicConstrained(i) = prop.isConstrainedSet && strcmpi(nm, prop.type);
67+
anon(i) = ~prop.isConstrainedSet && isempty(prop.name);
68+
69+
if ~isempty(prop.type)
70+
varnames{i} = nm;
71+
try
72+
typenames{i} = namespace.getFullClassName(prop.type);
73+
catch ME
74+
if ~strcmp(ME.identifier, 'NWB:Scheme:Namespace:NotFound')
75+
rethrow(ME);
76+
end
7577
end
7678
end
79+
elseif isa(prop, 'file.Attribute')
80+
isattr(i) = true;
7781
end
78-
elseif isa(prop, 'file.Attribute')
79-
isattr(i) = true;
8082
end
81-
end
8283

83-
%warn for missing namespaces/property types
84-
warnmsg = ['`' pname '`''s constructor is unable to check for type `%1$s` ' ...
85-
'because its namespace or type specifier could not be found. Try generating ' ...
86-
'the namespace or class definition for type `%1$s` or fix its schema.'];
84+
%warn for missing namespaces/property types
85+
warnmsg = ['`' pname '`''s constructor is unable to check for type `%1$s` ' ...
86+
'because its namespace or type specifier could not be found. Try generating ' ...
87+
'the namespace or class definition for type `%1$s` or fix its schema.'];
8788

88-
invalid = cellfun('isempty', typenames);
89-
invalidWarn = invalid & (dynamicConstrained | anon) & ~isattr;
90-
invalidVars = varnames(invalidWarn);
91-
for i=1:length(invalidVars)
92-
warning(warnmsg, invalidVars{i});
93-
end
94-
varnames = lower(varnames);
95-
96-
%we delete the entry in varargin such that any conflicts do not show up in inputParser
97-
deleteFromVars = 'varargin(ivarargin) = [];';
98-
%if constrained/anon sets exist, then check for nonstandard parameters and add as
99-
%container.map
100-
constrainedTypes = typenames(dynamicConstrained & ~invalid);
101-
constrainedVars = varnames(dynamicConstrained & ~invalid);
102-
methodCalls = strcat('[obj.', constrainedVars, ', ivarargin] = ',...
103-
' types.util.parseConstrained(obj,''', constrainedVars, ''', ''',...
104-
constrainedTypes, ''', varargin{:});');
105-
fullBody = cell(length(methodCalls) * 2,1);
106-
fullBody(1:2:end) = methodCalls;
107-
fullBody(2:2:end) = {deleteFromVars};
108-
fullBody = strjoin(fullBody, newline);
109-
bodystr(end+1:end+length(fullBody)+1) = [newline fullBody];
110-
111-
%if anonymous values exist, then check for nonstandard parameters and add
112-
%as Anon
113-
114-
anonTypes = typenames(anon & ~invalid);
115-
anonVars = varnames(anon & ~invalid);
116-
methodCalls = strcat('[obj.', anonVars, ',ivarargin] = ',...
117-
' types.util.parseAnon(''', anonTypes, ''', varargin{:});');
118-
fullBody = cell(length(methodCalls) * 2,1);
119-
fullBody(1:2:end) = methodCalls;
120-
fullBody(2:2:end) = {deleteFromVars};
121-
fullBody = strjoin(fullBody, newline);
122-
bodystr(end+1:end+length(fullBody)+1) = [newline fullBody];
123-
124-
parser = {...
125-
'p = inputParser;',...
126-
'p.KeepUnmatched = true;',...
127-
'p.PartialMatching = false;',...
128-
'p.StructExpand = false;'};
129-
130-
names = names(~dynamicConstrained & ~anon);
131-
defaults = cell(size(names));
132-
for i=1:length(names)
133-
prop = props(names{i});
134-
if (isa(prop, 'file.Group') &&...
135-
(prop.hasAnonData || prop.hasAnonGroups || prop.isConstrainedSet)) ||...
136-
(isa(prop, 'file.Dataset') && prop.isConstrainedSet)
137-
defaults{i} = 'types.untyped.Set()';
138-
else
139-
defaults{i} = '[]';
89+
invalid = cellfun('isempty', typenames);
90+
invalidWarn = invalid & (dynamicConstrained | anon) & ~isattr;
91+
invalidVars = varnames(invalidWarn);
92+
for i=1:length(invalidVars)
93+
warning(warnmsg, invalidVars{i});
14094
end
141-
end
142-
% add parameters
143-
parser = [parser, strcat('addParameter(p, ''', names, ''', ', defaults,');')];
144-
% parse
145-
parser = [parser, {'misc.parseSkipInvalidName(p, varargin);'}];
146-
% get results
147-
parser = [parser, strcat('obj.', names, ' = p.Results.', names, ';')];
148-
parser = strjoin(parser, newline);
149-
bodystr(end+1:end+length(parser)+1) = [newline parser];
95+
varnames = lower(varnames);
96+
97+
%we delete the entry in varargin such that any conflicts do not show up in inputParser
98+
deleteFromVars = 'varargin(ivarargin) = [];';
99+
%if constrained/anon sets exist, then check for nonstandard parameters and add as
100+
%container.map
101+
constrainedTypes = typenames(dynamicConstrained & ~invalid);
102+
constrainedVars = varnames(dynamicConstrained & ~invalid);
103+
methodCalls = strcat('[obj.', constrainedVars, ', ivarargin] = ',...
104+
' types.util.parseConstrained(obj,''', constrainedVars, ''', ''',...
105+
constrainedTypes, ''', varargin{:});');
106+
fullBody = cell(length(methodCalls) * 2,1);
107+
fullBody(1:2:end) = methodCalls;
108+
fullBody(2:2:end) = {deleteFromVars};
109+
fullBody = strjoin(fullBody, newline);
110+
bodystr(end+1:end+length(fullBody)+1) = [newline fullBody];
111+
112+
%if anonymous values exist, then check for nonstandard parameters and add
113+
%as Anon
114+
115+
anonTypes = typenames(anon & ~invalid);
116+
anonVars = varnames(anon & ~invalid);
117+
methodCalls = strcat('[obj.', anonVars, ',ivarargin] = ',...
118+
' types.util.parseAnon(''', anonTypes, ''', varargin{:});');
119+
fullBody = cell(length(methodCalls) * 2,1);
120+
fullBody(1:2:end) = methodCalls;
121+
fullBody(2:2:end) = {deleteFromVars};
122+
fullBody = strjoin(fullBody, newline);
123+
bodystr(end+1:end+length(fullBody)+1) = [newline fullBody];
124+
125+
parser = {...
126+
'p = inputParser;',...
127+
'p.KeepUnmatched = true;',...
128+
'p.PartialMatching = false;',...
129+
'p.StructExpand = false;'};
130+
131+
names = names(~dynamicConstrained & ~anon);
132+
defaults = cell(size(names));
133+
for i=1:length(names)
134+
prop = props(names{i});
135+
if (isa(prop, 'file.Group') &&...
136+
(prop.hasAnonData || prop.hasAnonGroups || prop.isConstrainedSet)) ||...
137+
(isa(prop, 'file.Dataset') && prop.isConstrainedSet)
138+
defaults{i} = 'types.untyped.Set()';
139+
else
140+
defaults{i} = '[]';
141+
end
142+
end
143+
% add parameters
144+
parser = [parser, strcat('addParameter(p, ''', names, ''', ', defaults,');')];
145+
% parse
146+
parser = [parser, {'misc.parseSkipInvalidName(p, varargin);'}];
147+
% get results
148+
parser = [parser, strcat('obj.', names, ' = p.Results.', names, ';')];
149+
parser = strjoin(parser, newline);
150+
bodystr(end+1:end+length(parser)+1) = [newline parser];
150151
end
151152

152153
function checkTxt = fillCheck(name, namespace)
153-
checkTxt = [];
154-
155-
% find if a dynamic table ancestry exists
156-
ancestry = namespace.getRootBranch(name);
157-
isDynamicTableDescendent = false;
158-
for iAncestor = 1:length(ancestry)
159-
ParentRaw = ancestry{iAncestor};
160-
% this is always true, we just use the proper index as typedefs may vary.
161-
typeDefInd = isKey(ParentRaw, namespace.TYPEDEF_KEYS);
162-
isDynamicTableDescendent = isDynamicTableDescendent ...
163-
|| strcmp('DynamicTable', ParentRaw(namespace.TYPEDEF_KEYS{typeDefInd}));
164-
end
154+
checkTxt = [];
155+
156+
% find if a dynamic table ancestry exists
157+
ancestry = namespace.getRootBranch(name);
158+
isDynamicTableDescendent = false;
159+
for iAncestor = 1:length(ancestry)
160+
ParentRaw = ancestry{iAncestor};
161+
% this is always true, we just use the proper index as typedefs may vary.
162+
typeDefInd = isKey(ParentRaw, namespace.TYPEDEF_KEYS);
163+
isDynamicTableDescendent = isDynamicTableDescendent ...
164+
|| strcmp('DynamicTable', ParentRaw(namespace.TYPEDEF_KEYS{typeDefInd}));
165+
end
165166

166-
if ~isDynamicTableDescendent
167-
return;
168-
end
167+
if ~isDynamicTableDescendent
168+
return;
169+
end
169170

170-
checkTxt = strjoin({ ...
171-
sprintf('if strcmp(class(obj), ''%s'')', namespace.getFullClassName(name)), ...
172-
' types.util.dynamictable.checkConfig(obj);', ...
173-
'end',...
174-
}, newline);
171+
checkTxt = strjoin({ ...
172+
sprintf('if strcmp(class(obj), ''%s'')', namespace.getFullClassName(name)), ...
173+
' types.util.dynamictable.checkConfig(obj);', ...
174+
'end',...
175+
}, newline);
175176
end

0 commit comments

Comments
 (0)