CINPLA / expipe-cinpla-legacy

All that was expipe and more
GNU General Public License v3.0
2 stars 2 forks source link

Filter options #7

Closed espenhgn closed 6 years ago

espenhgn commented 7 years ago

Allows setting a few klusta prm options from command line when invoking expipe process:

  --filter-order INTEGER          Butterworth filter order N. Default is N=3.
                                  For acausal filters (filter type
                                  "filtfilt"), the effective filter order is
                                  N*2
  --filter-function [filtfilt|lfilter]
                                  Filter function. The default "filtfilt"
                                  corresponds to a forward-backward filter
                                  operation, "lfilter" a forward filter. NOTE:
                                  does not affect filtering with klusta.
  --use-single-threshold [True|False]
                                  Use the same threshold across channels.
                                  Default is True
  --threshold-strong-std-factor FLOAT
                                  Strong spike detection threshold. Default is
                                  4.5
  --threshold-weak-std-factor FLOAT

Adds expipe transfer option:

  --override-prompts              disables yes/no prompts for automated
                                  removal of files after transfer. Default is
                                  False
lepmik commented 7 years ago

In stead of --override-prompts it is more conventional to have -y, --yes, which is what's used in the other modules as well. Do you agree to conform?

lepmik commented 7 years ago

Great with the filter options btw

espenhgn commented 6 years ago

Hi @lepmik I think --override-prompts sounds like a much less ambiguous argument than simply -y or --yes, as arguments can be supplied in an arbitrary order. But maybe you can point me to a place where your solution is used? Until then I do not agree to conform I guess.

Anyway think that some of the option names are poorly named, like --move could be renamed --remove-local or similar, but that is better dealt with independently.

espenhgn commented 6 years ago

Hi @lepmik Maybe we should just merge this in while there is no conflict, and rather argue about this override flag later.

lepmik commented 6 years ago

You can see here as an example on how we use a simple y, yes to override prompts elsewhere. I think it is important that the user do not have to look up every function to see how to override prompts for that specific function.

It is common to use a simple -y statement in e.g.

sudo apt -y install blah pip -y uninstall blah

espenhgn commented 6 years ago

Replaced --override-prompts with --yes option. Ok to merge?

lepmik commented 6 years ago

Great! Thank you for the update.