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

758 pspm_expand_epochs #786

Merged
merged 9 commits into from
Nov 19, 2024
Merged

758 pspm_expand_epochs #786

merged 9 commits into from
Nov 19, 2024

Conversation

4gwe
Copy link
Contributor

@4gwe 4gwe commented Sep 30, 2024

Fixes #758 .

Changes proposed in this pull request:

  • made pspm_expand_epochs
  • added to to pspm_remove

@4gwe 4gwe self-assigned this Sep 30, 2024
src/pspm_expand_epochs.m Outdated Show resolved Hide resolved
@@ -0,0 +1,147 @@
function [sts, ep_exp] = pspm_expand_epochs(epoches, expansion, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest using the varargin syntax.

% Initialize status
sts = -1;

% Check if epochs matrix and expansion vector are valid
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is needed for all modes of the function, I would check it in the beginning of the main function, before anything is loaded from file (faster and easier to read).

src/pspm_expand_epochs.m Outdated Show resolved Hide resolved

%
% Ensure that the start of any epoch is not negative
expanded_epochs_temp(expanded_epochs_temp(:,1) < 0, 1) = 0; % or <1???
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we should put this code into pspm_get_epochs (if the functionality does not exist there already), because it is needed in several functions.

then just call [sts, epochs] = pspm_get_timing('epochs', epochs)

@@ -12,14 +12,17 @@
% be in seconds. This parameter is passed to pspm_get_timing().
% * timeunits: timeunits of the epochfile.
% ┌───options
% └─.channel_action:
% └─.channel_action: [pre, post]
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the channel action changed?

@4gwe 4gwe added the Completed & Waiting for Review Completed and waiting for review label Oct 7, 2024
Copy link
Contributor

@dominikbach dominikbach left a comment

Choose a reason for hiding this comment

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

some functions return sts == -1 if unsuccessful, others sts == 0. So to be most general, it is best practice to check for unsuccessful downstream functions with if sts < 1

src/pspm_expand_epochs.m Outdated Show resolved Hide resolved
@dominikbach dominikbach merged commit 73f24dc into develop Nov 19, 2024
1 check passed
@dominikbach dominikbach deleted the 758-pspm_expand_epochs branch November 19, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Completed & Waiting for Review Completed and waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a function to expand epochs
2 participants