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.

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

ENH - make some of the code more robust for string input #2383

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions fileio/ft_filetype.m
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@
end
end

% some of the code does not work well on matlab strings, (i.e. "" vs ''),
% specifically ["a" "b"] yields something different than ['a' 'b'].
if isstring(filename)
filename = char(filename);
end

% the parts of the filename are used further down
if isfolder(filename)
[p, f, x] = fileparts(filename);
Expand Down
6 changes: 6 additions & 0 deletions fileio/ft_read_headshape.m
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@
return
end % if iscell

% some of the code does not work well on matlab strings, (i.e. "" vs ''),
% specifically ["a" "b"] yields something different than ['a' 'b'].
if isstring(filename)
filename = char(filename);
end

% optionally get the data from the URL and make a temporary local copy
filename = fetch_url(filename);

Expand Down
2 changes: 2 additions & 0 deletions ft_defaults.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
% ft_default.checkconfig = string, can be 'pedantic', 'loose', 'silent' (default = 'loose')
% ft_default.checkpath = string, can be 'pedantic', 'once', 'no' (default = 'pedantic')
% ft_default.checksize = number in bytes, can be inf (default = 1e5)
% ft_default.checkstring = string, can be 'yes' or 'no' (default = 'yes'), convert "strings" in cfg to 'chars'
% ft_default.showlogo = string, can be 'yes' or 'no' (default = 'yes')
% ft_default.showcallinfo = string, can be 'yes' or 'no' (default = 'yes')
% ft_default.trackcallinfo = string, can be 'yes' or 'no' (default = 'yes')
Expand Down Expand Up @@ -98,6 +99,7 @@
if ~isfield(ft_default, 'checkconfig'), ft_default.checkconfig = 'loose'; end % pedantic, loose, silent
if ~isfield(ft_default, 'checkpath'), ft_default.checkpath = 'pedantic'; end % pedantic, once, no
if ~isfield(ft_default, 'checksize'), ft_default.checksize = 1e5; end % number in bytes, can be inf
if ~isfield(ft_default, 'checkstring'), ft_default.checkstring = 'yes'; end % yes or no
if ~isfield(ft_default, 'showlogo'), ft_default.showlogo = 'yes'; end % yes or no, this is relevant for SPM and EEGLAB
if ~isfield(ft_default, 'showcallinfo'), ft_default.showcallinfo = 'yes'; end % yes or no, this is used in ft_pre/postamble_provenance
if ~isfield(ft_default, 'keepprevious'), ft_default.keepprevious = 'yes'; end % yes, no, this is used in ft_postamble_previous
Expand Down
46 changes: 46 additions & 0 deletions test/test_issue2082.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
function test_issue2082

% MEM 1gb
% WALLTIME 00:30:00
% DEPENDENCY ft_checkconfig
% DATA public

global ft_default

cfg = [];
cfg.dataset = string(dccnpath('/home/common/matlab/fieldtrip/data/ftp/test/ctf/Subject01.ds'));
cfg = ft_checkconfig(cfg, 'dataset2files', 'yes');

filename = string(dccnpath('/home/common/matlab/fieldtrip/data/ftp/test/ctf/Subject01.shape'));
hs = ft_read_headshape(filename);

% create a dummy cfg
cfg.a = 'a';
cfg.b = "b";
cfg.c = 1;
cfg.d.e = "e";
cfg.d.f = 1;
cfg.d.g.h = 'h';
cfg.d.g.i = "i";
cfg.d.g.j = {"a" "b"};
k.a = "a";
k.b = 'b';
cfg.d.g.k = {k 1}; % this for now does not work

cfg.checkstring = 'no';
cfgout1 = ft_checkconfig(cfg);
c = struct2cell(cfgout1);
s = fieldnames(cfg);
t = cellfun(@class, c, 'UniformOutput', false);
assert(any(strcmp(t, 'string')));

cfg.checkstring = 'yes';
cfgout2 = ft_checkconfig(cfg);

c = struct2cell(cfgout2);
t = cellfun(@class, c, 'UniformOutput', false);
assert(~any(strcmp(t, 'string')));

c = struct2cell(cfgout2.d);
t = cellfun(@class, c, 'UniformOutput', false);
assert(~any(strcmp(t, 'string')));
46 changes: 46 additions & 0 deletions utilities/ft_checkconfig.m
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@
pedantic = false;
end

if isfield(cfg, 'checkstring')
checkstring = cfg.checkstring;
else
checkstring = 'no';
end

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% rename old to new options, give warning
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -607,6 +613,17 @@
end
end


%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% checkstring, i.e. convertStringsToChar
%
% Converts "strings" to 'chars' if necessary.
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

if istrue(checkstring)
cfg = checkstringfun(cfg);
end

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% checkinside, i.e. inside2logical
%
Expand Down Expand Up @@ -750,6 +767,35 @@
end % for numel(cfg)
end % for each of the fieldsorig

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% SUBFUNCTION
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
function [cfg] = checkstringfun(cfg)

c = struct2cell(cfg);
s = fieldnames(cfg);
t = cellfun(@class, c, 'UniformOutput', false);

% convert
[c{:}] = convertStringsToChars(c{:});
cfg = cell2struct(c,s,1);

% deal with cell-arrays
if any(strcmp(t, 'cell'))
fn = s(strcmp(t, 'cell'));
for k = 1:numel(fn)
[cfg.(fn{k}){:}] = convertStringsToChars(cfg.(fn{k}){:});
end
end

% recurse
if any(strcmp(t, 'struct'))
fn = s(strcmp(t,'struct'));
for k = 1:numel(fn)
cfg.(fn{k}) = checkstringfun(cfg.(fn{k}));
end
end

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% SUBFUNCTION converts a cell-array of structure arrays into a structure array
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down
5 changes: 5 additions & 0 deletions utilities/private/dataset2files.m
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
return
end

if isstring(filename)
% the below code does not deal well with matlab strings (i.e. "" vs. '')
filename = char(filename);
end

if isempty(format)
format = ft_filetype(filename);
end
Expand Down