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

[ENH] - Add average_reconstructions function #289

Merged
merged 3 commits into from
Jul 21, 2023
Merged

[ENH] - Add average_reconstructions function #289

merged 3 commits into from
Jul 21, 2023

Conversation

TomDonoghue
Copy link
Member

Adds a new average_reconstructions function as a minor helper function - an addition mentioned in #227

Note: the last commit is unrelated to the new function - it's just a small update to the aliasing of plot_spectrum to plot_spectra - moving this alias to the file means that from fooof.plts.spectra import plot_spectrum should work now whereas is would fail otherwise.

@TomDonoghue TomDonoghue added the 1.1 Targetted for a fooof 1.1 release. label Jul 18, 2023
@TomDonoghue TomDonoghue mentioned this pull request Jul 18, 2023
3 tasks
@fooof-tools fooof-tools deleted a comment from codecov-commenter Jul 18, 2023
Copy link
Contributor

@ryanhammonds ryanhammonds left a comment

Choose a reason for hiding this comment

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

Looks good! Left a minor comment about duplicate checks that can be ignored


avg_funcs = {'mean' : np.nanmean, 'median' : np.nanmedian}
if avg_method not in avg_funcs.keys():
raise ValueError("Requested average method not understood.")
Copy link
Contributor

Choose a reason for hiding this comment

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

The above checks are duplicated in average_fg and could be moved to a shared check func.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeh, I agree! I didn't go for that here, because there are related changes in #283 that add a general function for averaging data, but I didn't bother trying to backport that, and so the plan is to consolidate this into the 2.0 release and clean up the code here when merging!

@TomDonoghue TomDonoghue merged commit 3635f34 into main Jul 21, 2023
6 checks passed
@TomDonoghue TomDonoghue deleted the avgg branch July 21, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.1 Targetted for a fooof 1.1 release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants