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

Unify function output #689

Merged
merged 37 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5e40e47
modify pspm_combine_markerchannels
Apr 12, 2024
a803376
modify pspm_convert_au2unit and pspm_convert_area2diameter
Apr 12, 2024
9b391cc
fix pspm_load_channel
Apr 12, 2024
cb6813c
modify pspm_convert_hb2hp
Apr 12, 2024
9e5d630
modify pspm_convert_ppg2hb & pspm_emg_pp
Apr 12, 2024
2608a84
modify pspm_find_sounds
Apr 12, 2024
3ecf795
modify pspm_pupil_correct_eyelink
Apr 13, 2024
25345c1
modify pspm_resp_pp
Apr 13, 2024
545af94
modify pspm_scr_pp
Apr 13, 2024
d69277b
modify pspm_pp
Apr 13, 2024
5e973c7
modify pspm_merge
Apr 14, 2024
48a9edb
modify pspm_split_sessions
Apr 14, 2024
9d62414
modify pspm_remove_epochs
Apr 14, 2024
8b092c1
modify pspm_trim
Apr 15, 2024
afc20b9
modify pspm_import
Apr 16, 2024
ffe2f70
modify pspm_dcm
Apr 16, 2024
ae4a435
modify pspm_dcm_inv
Apr 16, 2024
ba1d6ff
modify pspm_glm
Apr 16, 2024
91d3fca
remove varargout in pspm_sf_... and pspm_transfer_function
Apr 16, 2024
72eaf15
modify pspm_version
Apr 16, 2024
06bad35
fixes after testing
Apr 18, 2024
cb29ec1
fixes after testing
Apr 18, 2024
31618c4
Merge branch 'develop' into unify-function-output
teddychao Apr 19, 2024
f2225b9
Merge branch 'develop' into unify-function-output
dominikbach Apr 19, 2024
c88cca3
modify pspm_interpolate
Apr 19, 2024
94cfdc8
improve pspm_select_channels
Apr 19, 2024
109035e
test fix
Apr 19, 2024
6b9c365
test fix
Apr 19, 2024
2dbd2fb
remove comment numbering in pspm_check_data
May 14, 2024
4969a2b
fix pspm_cfg_rename
May 14, 2024
ef8241b
fix pspm_cfg_find_sounds
May 15, 2024
b7fdc3e
Merge branch 'develop' into unify-function-output
dominikbach May 20, 2024
5a8a12b
fix pspm_version and GUI for pspm_merge
May 21, 2024
bd999ac
fix pspm_rename and GUI item
May 21, 2024
9f8a9b4
improve test for gaze_convert
May 21, 2024
74ceee8
Merge branch 'unify-function-output' of github.com:bachlab/PsPM into …
May 21, 2024
17e42f1
fix test for pspm_rename
May 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 10 additions & 15 deletions src/pspm_cfg/pspm_cfg_merge.m
dominikbach marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,19 @@
first_file = cfg_files;
first_file.name = 'First file(s)';
first_file.tag = 'first_file';
first_file.num = [1 Inf];
first_file.num = [1 1];
first_file.help = {['Specify the first of the two files to be ', ...
'merged. This can be one file or a set of files. The output file ', ...
'merged. The output file ', ...
'will have the name of the first file prepended with an ''m''.'],...
' ',settings.datafilehelp};

%% Second file
second_file = cfg_files;
second_file.name = 'Second file(s)';
second_file.tag = 'second_file';
second_file.num = [1 Inf];
second_file.help = {['Specify the second of the two files to be merged. ', ...
'This can be one file or a set of files. This set must have the same ', ...
'length as the set defined as first file(s).'],' ',settings.datafilehelp};
second_file.num = [1 1];
second_file.help = {['Specify the second of the two files to be merged. '], ...
' ', settings.datafilehelp};

%% Data files
datafiles = cfg_branch;
Expand Down Expand Up @@ -66,8 +65,8 @@
marker_chan.val = {[0 0]};
marker_chan.num = [1 2];
marker_chan.strtype = 'i';
marker_chan.help = {['Specify for each file (first and second) a ', ...
'channel which should be used as marker reference. A 1x2 vector is ', ...
marker_chan.help = {['Specify for both files a numerical ', ...
'channel index, which should be used as marker reference. A 1x2 vector is ', ...
'expected. If equal to 0, the first marker channel is used. ', ...
'Default: [0 0]']};

Expand All @@ -86,14 +85,10 @@
merge.prog = @pspm_cfg_run_merge;
merge.vout = @pspm_cfg_vout_merge;
merge.help = {['Allows to merge (i.e. stack) two files that were acquired ', ...
'simultaneously but contain different channels. Multiple files are ', ...
'allowed and are processed in a sequential manner. This means the ', ...
'first element of second set of files is merged into the first ', ...
'element of first set of files and so on. Therefore first set and ', ...
'second set must have the same number of elements. The files are ', ...
'simultaneously but contain different channels. The files are ', ...
'aligned according to the first event of a given marker channel or ', ...
'to the start of the file. The output file consists of an ''m'' at ', ...
'the beginning and the name of the first file appended. ']};
'to the start of the file. The output file name consists of an ''m'' ', ...
'prepended to the name of the first file. ']};

function vout = pspm_cfg_vout_merge(job)
vout = cfg_dep;
Expand Down
5 changes: 2 additions & 3 deletions src/pspm_cfg/pspm_cfg_pp_scr.m
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,9 @@
scr_pp_options.channel_action = job.chan_action;
[sts, output] = pspm_scr_pp(scr_pp_datafile, scr_pp_options);
if sts == 1
% out = {output.channel};
out = output{1};
out = output;
else
out = {-1};
out = -1;
end
end

Expand Down
12 changes: 3 additions & 9 deletions src/pspm_cfg/pspm_cfg_rename.m
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
filename.name = 'File Name';
filename.tag = 'filename';
filename.num = [1 1];
%filename.filter = '\.mat$';
filename.filter = '\.mat$';
filename.help = {'Choose name of original file.'};

newfilename = cfg_entry;
newfilename.name = 'New File Name';
newfilename.tag = 'newfilename';
newfilename.strtype = 's';
%newfilename.num = [1 1];
newfilename.num = [1 1];
teddychao marked this conversation as resolved.
Show resolved Hide resolved
newfilename.help = {''};

file = cfg_branch;
Expand All @@ -24,17 +24,11 @@
file.val = {filename,newfilename};
file.help = {''};

rename_file = cfg_repeat;
rename_file.name = 'Rename';
rename_file.tag = 'rename_file';
rename_file.values = {file};
rename_file.help = {'Choose how many files to rename.'};

%% Executable branch
rename = cfg_exbranch;
rename.name = 'Rename File';
rename.tag = 'rename';
rename.val = {rename_file};
rename.val = {filename, newfilename};
rename.prog = @pspm_cfg_run_rename;
rename.vout = @pspm_cfg_vout_rename;
rename.help = {'Rename PsPM data file. This renames the file and updates the file information.'};
Expand Down
13 changes: 2 additions & 11 deletions src/pspm_cfg/pspm_cfg_run_dcm.m
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,5 @@
if eventnameflag
options.eventnames = eventnames;
end
[varargout] = pspm_dcm(model, options);
if numel(varargout) < 2
out = varargout;
dcm = [];
else
out = varargout{1};
dcm = varargout{2};
end
if ~iscell(out)
out = {out};
end
[out, dcm] = pspm_dcm(model, options);

6 changes: 1 addition & 5 deletions src/pspm_cfg/pspm_cfg_run_find_sounds.m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Line 24--27 may not work as fully expected, because the channel action does not have a fallback. Users cannot avoid creating a channel. To fix this, I suggest to change line 24--27 to be

if isfield(d.create_corrected_chan, 'yes')
  options.channel_action = 'add';
  options.channel_output = 'corrected';
else
  options.channel_action = 'none';
end

And in pspm_options, it should be modified in line 217, to be

options = autofill_channel_action(options,            'add',     {'replace', 'none'} );

Copy link
Contributor Author

@dominikbach dominikbach May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new design is to always create a channel (i.e. never give the GUI user the option not to). But this was incorrectly implemented in the GUI. It should be fixed now.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
options.roi = job.roi.region;
end
options = pspm_update_struct(options, job, 'threshold');
options.channel_action = 'none';
f = fieldnames(job.output);
switch f{1}
case 'create_chan'
Expand Down Expand Up @@ -38,8 +37,5 @@
case 'text_only'
options.plot = false;
end
[~, infos] = pspm_find_sounds(file, options);
if isfield(infos, 'channel')
out = infos.channel;
end
[~, out, infos] = pspm_find_sounds(file, options);
end
2 changes: 1 addition & 1 deletion src/pspm_cfg/pspm_cfg_run_import.m
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,4 @@
end
options = struct();
options = pspm_update_struct(options, job, 'overwrite');
out = pspm_import(datafiles, datatype, import, options);
[sts, out] = pspm_import(datafiles, datatype, import, options);
6 changes: 1 addition & 5 deletions src/pspm_cfg/pspm_cfg_run_merge.m
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,4 @@
options.marker_chan_num = job.options.marker_chan;
end
% run merge
[out] = pspm_merge(infile1, infile2, ref, options);
% ensure output is always a cell
if ~iscell(out)
out = {out};
end
[sts, out] = pspm_merge(infile1, infile2, ref, options);
4 changes: 2 additions & 2 deletions src/pspm_cfg/pspm_cfg_run_pp_emg_data.m
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
options.mains_freq = job.options(1).mains_freq;
options.channel_action = job.options(1).chan_action;
options.channel = pspm_cfg_channel_selector('run', job.options(1).chan);
[sts, output] = pspm_emg_pp(job.datafile{1}, options);
[sts, outchannel] = pspm_emg_pp(job.datafile{1}, options);
if sts == 1
out = {output.channel};
out = {outchannel};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw other functions, such as pspm_cfg_run_merge has removed out={out} if it is not a cell. I am not sure if this is still necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GUI dependencies will be updated in a future PR.

else
out = {-1};
end
16 changes: 8 additions & 8 deletions src/pspm_cfg/pspm_cfg_run_pp_heart_data.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@
{'semi', 'twthresh'});
options = pspm_update_struct(options, job, 'channel_action');
% call function
[sts, winfo] = pspm_convert_ecg2hb(fn, options);
[sts, outchannel] = pspm_convert_ecg2hb(fn, options);
case 'ecg2hb_amri'
options = pp_field.opt;
options.channel = chan;
options = pspm_update_struct(options, job, 'channel_action');
winfo = struct();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this line can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

[sts, winfo.channel] = pspm_convert_ecg2hb_amri(fn, options);
[sts, outchannel] = pspm_convert_ecg2hb_amri(fn, options);
case 'hb2hp'
sr = job.pp_type{i}.hb2hp.sr;
options = struct();
options.channel = chan;
options.limit = job.pp_type{i}.hb2hp.limit;
options = pspm_update_struct(options, job, 'channel_action');
[sts, winfo] = pspm_convert_hb2hp(fn, sr, options);
[sts, outchannel] = pspm_convert_hb2hp(fn, sr, options);
case 'ecg2hp'
sr = job.pp_type{i}.ecg2hp.sr;
% copy options
Expand All @@ -51,16 +51,16 @@
options = pspm_update_struct(options, job, {'channel_action'});
options.channel = chan;
% call ecg2hb
[sts, winfo] = pspm_convert_ecg2hb(fn, options);
[sts, outchannel] = pspm_convert_ecg2hb(fn, options);
if sts ~= -1
% replace channel
options.channel_action = 'replace';
options = pspm_update_struct(options, ...
job.pp_type{i}.ecg2hp, ...
'limit');
options.channel = winfo.channel;
options.channel = outchannel;
% call ecg2hp
[sts, winfo] = pspm_convert_hb2hp(fn, sr, options);
[sts, outchannel] = pspm_convert_hb2hp(fn, sr, options);
end
case 'ppg2hb'
options = struct();
Expand All @@ -73,10 +73,10 @@
end
options.channel = chan;
options = pspm_update_struct(options, job, {'channel_action'});
[sts, winfo] = pspm_convert_ppg2hb(fn, options);
[sts, outchannel] = pspm_convert_ppg2hb(fn, options);
end
if sts ~= -1
outputs{i} = winfo.channel;
outputs{i} = outchannel;
else
outputs{i} = [];
warning('Error occured during conversion. Could not finish correctly.');
Expand Down
2 changes: 1 addition & 1 deletion src/pspm_cfg/pspm_cfg_run_rename.m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this file is working. I checked and saw it reported error when I tried to rename a file. The job.file did not exist in this occasion. My understanding of what has been updated to pspm_cfg_run_merge is to make it consistent to the output from other functions, so it is not expecting to change the functionality of "rename". Maybe you could have a look and see if any unexpected update has caused this issue?
Screenshot 2024-05-13 at 15 01 47

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is fixed now.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
filename{i} = job.file(i).filename{1};
newfilename{i} = job.file(i).newfilename;
end
out = pspm_ren(filename, newfilename);
[sts, out] = pspm_ren(filename, newfilename);
if ~iscell(out)
out = {out};
end
6 changes: 2 additions & 4 deletions src/pspm_cfg/pspm_cfg_run_trim.m
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,5 @@
if ~isfield(options,'marker_chan_num')
options.marker_chan_num = 0; % Default value
end
out = pspm_trim(job.datafile, from, to, ref, options);
if ~iscell(out)
out = {out};
end
[sts, out] = pspm_trim(job.datafile{1}, from, to, ref, options);

5 changes: 1 addition & 4 deletions src/pspm_cfg/pspm_cfg_split_sessions.m
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@
elseif isfield(job.split_behavior, 'marker')
options.splitpoints = job.split_behavior.marker;
end
out = pspm_split_sessions(datafile, options);
if ~iscell(out)
out = {out};
end
[sts, out] = pspm_split_sessions(datafile, options);
end
end
4 changes: 2 additions & 2 deletions src/pspm_cfg/pspm_cfg_trim.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

%% Select file
datafile = cfg_files;
datafile.name = 'Data File(s)';
datafile.name = 'Data File';
datafile.tag = 'datafile';
datafile.num = [1 Inf];
datafile.num = [1 1];
datafile.help = {'Select datafile.',' ',settings.datafilehelp};

%% Marker channel number
Expand Down
119 changes: 119 additions & 0 deletions src/pspm_check_data.m
teddychao marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
function [sts, data] = pspm_check_data(data, infos)
% ● Description
% pspm_checks_data checks a PsPM data (and, optional, infos) structure
% This function is used throughout PsPM (and in particular in
% pspm_load_data). It returns a data cell array with legacy and format
% conversions.
% ● Format
% [sts, data] = pspm_check_data(data, infos)

% this code is taken from pspm_load_data; it could be improved using
% cellfun

global settings
if isempty(settings)
pspm_init;
end
sts = -1;

if nargin < 1
warning('ID:invalid_input', 'No input - don''t know what to do');
return
end

% check infos
if nargin > 1
flag_infos = 0;
if isempty(fieldnames(infos))
flag_infos = 1;
else
if ~isfield(infos, 'duration')
flag_infos = 1;
end
end
if flag_infos
warning('ID:invalid_data_structure', 'Input data does not have sufficient infos');
return
end
else
flag_infos = 1;
end

% 7.1 initialise error flags --
vflag = zeros(numel(data), 1); % records data structure, valid if 0
wflag = zeros(numel(data), 1); % records whether data is out of range, valid if 0
nflag = zeros(numel(data), 1);

% loop through channels
for k = 1:numel(data)
% 7.2 Check header --
if ~isfield(data{k}, 'header')
vflag(k) = 1;
else
% 7.2.1 Convert header channeltype into chantype if there are --
if isfield(data{k}.header, 'channeltype')
data{k}.header.chantype = data{k}.header.channeltype;
data{k}.header = rmfield(data{k}.header, 'channeltype');
end
if ~isfield(data{k}.header, 'chantype') || ...
~isfield(data{k}.header, 'sr') || ...
~isfield(data{k}.header, 'units')
vflag(k) = 1;
else
if ~ismember(lower(data{k}.header.chantype), {settings.channeltypes.type})
nflag(k) = 1;
end
end
end
% 7.3 Check data --
if vflag(k)==0 && nflag(k)==0
% required information is available and valid in header and infos
if ~isfield(data{k}, 'data')
vflag(k) = 1;
else
if isempty(data{k}.data)
warning('ID:missing_data', 'Channel %01.0f is empty.', k);
% convert empty data to a generalised 1-by-0 matrix
data{k}.data = zeros(1,0);
elseif ~isvector(data{k}.data)
vflag(k) = 1;
elseif size(data{k}.data, 1) < size(data{k}.data, 2)
data{k}.data = data{k}.data(:);
warning('ID:invalid_data_structure', ...
'Channel %i seems to have the wrong orientation. Trying to transpose...', k);
end
if ~flag_infos && ~vflag(k)
if strcmpi(data{k}.header.units, 'events')
if (any(data{k}.data > infos.duration) || any(data{k}.data < 0))
wflag(k) = 1;
end
else
if (length(data{k}.data) < infos.duration * data{k}.header.sr - 3 ||...
length(data{k}.data) > infos.duration * data{k}.header.sr + 3)
wflag(k) = 1;
end
end
end
end
end
end

if any(vflag)
errmsg = [sprintf('Invalid data structure for channel %01.0f.', find(vflag,1))];
warning('ID:invalid_data_structure', '%s', errmsg);
return
end
if any(wflag)
errmsg = [sprintf(['The data in channel %01.0f is out of ',...
'the range [0, infos.duration]'], find(wflag,1))];
warning('ID:invalid_data_structure', '%s', errmsg);
return
end
if any(nflag)
errmsg = [sprintf('Unknown channel type %s in channel %01.0f', data{find(nflag,1)}.header.chantype, find(nflag,1))];
warning('ID:invalid_data_structure', '%s', errmsg);
return
end

sts = 1;

Loading
Loading