Skip to content

Conversation

@scott-huberty
Copy link
Member

Request for Comment

This is another issue-as-PR, with changes in this PR serving as a demonstration of the proposed solution.

For optional Pipeline steps like find_breaks, and flag_channels_fixed_threshold, I'm trying to think of a way to.

  1. Make these steps more discoverable to users
  2. Make it easier for users to run these steps with default values
  3. Make it easier for users to configure these steps.
  4. Hopefully(!) Standardize the way we handle these steps in the config.

The current behavior

find_breaks.

find_breaks is not in our default configs. Thus the user must know that this step is an option, and how to add it to the config... E.g. they have to know to add either...

find_breaks: {}

or

find_breaks: {"threshold": .0005}

to the config. They can't just add this step as an empty key to the config, like

find_breaks:

Because the pipeline doesn't currently know how to handle this.

flag_channels_fixed_threshold.

This is not in our default configs. The user must know that this step is an option, and how to add it to the config. Unlike find_breaks however, simply adding this step as an empty key DOES work, e.g.:

flag_channels_fixed_threshold:

will run this step with the default threshold value.

Proposal

We add these optional pipeline steps as keys to our default configs, but without any values assigned to them, e.g.

find_breaks: # {}
flag_channels_fixed_threshold: # {}
flag_epochs_fixed_threshold: # {}

I propose that the pipeline skip any step that exists as empty keys in the config.. thus, these steps will be skipped by default.

  • Using find_breaks as an example, If the users want to run the step with the default values, they can do
find_breaks: {}
  • The user can further configure this step by either using flow-style syntax, e.g.
find_breaks: {min_break_duration: 5}

OR block-style syntax:

find_breaks:
  min_break_duration: 5

I'm cautiously optimistic that this is an intuitive approach to our config. But comments are welcome and requested!

@christian-oreilly
Copy link
Member

I am fine with what is proposed, with the understanding that

find_breaks: # {}
flag_channels_fixed_threshold: # {}
flag_epochs_fixed_threshold: # {}

is optional if a user don't want to run these steps. With respect to point 1 ("Make these steps more discoverable to users"), we may want to eventually make available some method or attributes (e.g., pipeline.AVAILABLE_STEPS and pipeline.used_steps).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants