-
Notifications
You must be signed in to change notification settings - Fork 102
[ENH] Enable full plot range for fm.plot() #246
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] Enable full plot range for fm.plot() #246
Conversation
Hey @moritz-gerster - can you merge main branch into your branch? There was an issue with the CI, so I made some maintenance updates to get things back up and running. |
Hey @TomDonoghue, I synced my fork, it should now be up to date! |
@TomDonoghue let me know if I should change something about the PR such as for example adding some tests 👍 |
Hey @TomDonoghue, I'm happy to add some tests to the PR if you would consider it useful :) |
Hey @moritz-gerster - thanks for opening a PR on this idea, and sorry for the delay - things have been busy, but we're picking up on some fooof development now. I added a broader set of notes on this topic in #245, and offered a different strategy in #276. Let me know what you think of that - if it looks good to you, it can replace this, and keep the object more lightweight. By the way, in terms of implementation, if we were to go this direction (storing the full spectrum in the object), I think that we would actually want to store a single copy of the broad range spectrum, and do something like use a masked array, or do an on-the-fly computation for extracting the fit range, rather than duplicating the arrays. |
Agreed :) |
Confirming - following discussion in #245 and #276, a different strategy to allow for the same functionality has now been merged in #276, so closing this PR! Thanks for the work @moritz-gerster - it helped get a cool new feature into the module! |
Fixes #245. With the new functionality, an inspection of fits with a narrow fitting range will be much more user-friendly.
This kind of inspection was recommended in Gerster et al. (2022): Challenge 2: Avoiding Oscillations Crossing Fitting Range Borders.
For new behavior see the last two figures here: Full_Plot_Range.