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

[BUG] IndexError when plotting #211

Closed
ryanhammonds opened this issue May 4, 2021 · 5 comments
Closed

[BUG] IndexError when plotting #211

ryanhammonds opened this issue May 4, 2021 · 5 comments

Comments

@ryanhammonds
Copy link
Contributor

This issue is came up here. Whenever no peaks are found and plot_peaks='line' is passed to the plot method of a fm, an IndexError is raised. It may be useful turn this error into a warning?

@TomDonoghue
Copy link
Member

Is this still the case on main, or just 1.0.0? I think I fixed this recently. If it's still happening, then yeh, I don't think it should even be a warning, it should just proceed as normal, without annotating any peaks since there aren't any.

@ryanhammonds
Copy link
Contributor Author

I tried from main too, and the same error was raised.

@TomDonoghue
Copy link
Member

TomDonoghue commented May 5, 2021

Ahh, it looks like I fixed it somewhere in #205 then, as this seems to work now on that branch.

Just to double check - this is what I ran:

from specparam import PSD
from specparam.sim import gen_power_spectrum

freqs, pows = gen_power_spectrum([3, 40], [1, 1], [1, 1, 1])

fm = PSD(max_n_peaks=0)
fm.fit(freqs, pows)
fm.plot(plot_peaks='line')

Which (on name) runs, and spits out a plot with no annotated peaks (since there are none) and no warning or error.
Does this seem like it addresses the error, and seem like good behaviour to you?

Sidenote: if I remember right, fixing this ended up being cleaning up how gauss_params_ gets initialized (or similar), so if it's empty it plays well with downstream processes like grabbing peaks for plotting. I don't think there are any relevant changes to the actual plot functions (in case you are having a look at what got changed).

@ryanhammonds
Copy link
Contributor Author

I was wrong and this is fixed on main too! I was getting an unrelated error from using plot_style=None which seems to be deprecated on main.

Since this is already fixed, I'll go ahead and close this.

@TomDonoghue
Copy link
Member

Ahh, okay cool that makes sense! I think the plot_style is deprecated since #176 got merged. Anyways, glad it should be fixed now - thanks for checking through!

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

No branches or pull requests

2 participants