Skip to content

Conversation

@TomDonoghue
Copy link
Member

@TomDonoghue TomDonoghue commented Mar 9, 2019

Responds to #131

Notable changes (in code):

  • Public setting min_peak_amplitude -> min_peak_height
  • Private setting _ap_amp_thresh -> _ap_percentile_thresh
  • Reference to 'Amp' as a usable label to select data -> 'PW' (for power)

Notable changes (in documentation / tutorials):

  • Where possible, most references to 'amplitude' things have been made agnostic, referring to 'height' instead of using the words 'amplitude' or 'power'.
  • Where in reference to extracted peaks in particular, the word 'power', with abbreviation 'PW' is used.
    This is in context of labelling peaks as {'CF': center frequency, 'PW': power, 'BW': bandwidth}
  • Through doing this, I noticed some inconsistencies of when things are referred to as 'peaks' vs 'gaussians', so some updates (in documentation / description) related to that as well.

Open Question:

  • I'm not super convinced about using 'PW' for 'power'. It has a nice symmetry of indexing using 'CF', 'PW' and 'BW'. Most likely alternative is probably 'POW'.
    • ^This should be a pretty clean find & replace to update / change, if anyone has suggestions / votes on a change to this.

Copy link
Contributor

@voytek voytek left a comment

Choose a reason for hiding this comment

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

just one comment

@voytek
Copy link
Contributor

voytek commented Mar 9, 2019

PW is fine. Don't overthink it too much.

@fooof-tools fooof-tools deleted a comment from codecov-io Mar 12, 2019
@fooof-tools fooof-tools deleted a comment from codecov-io Mar 12, 2019
@TomDonoghue TomDonoghue requested a review from rdgao March 12, 2019 23:36
@TomDonoghue TomDonoghue added the 1.0 Should go into FOOOF 1.0 release label Mar 13, 2019
@TomDonoghue TomDonoghue changed the base branch from set_avg to master March 20, 2019 03:00
@fooof-tools fooof-tools deleted a comment from codecov-io Mar 20, 2019
@TomDonoghue TomDonoghue merged commit dcc93b1 into master Mar 20, 2019
@TomDonoghue TomDonoghue deleted the amp branch March 20, 2019 03:06
@fooof-tools fooof-tools deleted a comment from codecov-io Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.0 Should go into FOOOF 1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants