-
Notifications
You must be signed in to change notification settings - Fork 100
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] Group shading #199
[ENH] Group shading #199
Conversation
I updated this func with some of the changes we chatted about recently including moving, renaming (though I'm still not sure about the name), and updating approaches for calculate average and shade. @ryanhammonds - can you check the updates, and see if this looks good to you, and test it out that everything works? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments that I'm going to push updates for. If you agree with the comments/changes then this should be ready to merge. The changes were related to getting custom callables working:
def average_callable(powers):
return np.mean(powers, axis=0)
def shade_callable(powers):
return np.std(powers, axis=0)
plot_spectra_yshade(freqs, powers, shade=shade_callable,
average=average_callable, log_powers=True)
fooof/plts/spectra.py
Outdated
# Organize mean spectrum to plot | ||
avg_funcs = {'mean' : np.mean, 'median' : np.median} | ||
avg_func = avg_funcs[average] if isinstance(average, str) else average | ||
avg_powers = avg_func(plt_powers, axis=0) if plt_powers.ndim == 2 else plt_powers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will require an average callable to have an axis arg. A custom callable may not have this, but np functions will. We may just want to require an array of length equal to the first dimension of plt_powers to be returned?
If a numpy callable is expected, we should make a note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh, great points. I think what you've done makes a ton of sense to address this.
@ryanhammonds - thanks for the review here! You are totally correct, I totally dropped the ball on actually implementing callable support lol (oops - too much late night coding). I like your updates here - merging this in. |
This is a point in #193 and adds shading around group spectra. By default, the shading defaults to +/- 1 stdev. This is bypassed if a 1d array is passed to
shade
.