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

[MNT] - Make grid an updateable argument in PSD plots #300

Merged
merged 7 commits into from
Sep 13, 2023
Merged

Conversation

TomDonoghue
Copy link
Member

Minor update to managing style of PSD plots to allow for passing in and controlling whether there is a grid or not.

@fooof-tools fooof-tools deleted a comment from codecov-commenter Aug 27, 2023
@TomDonoghue TomDonoghue added the 1.1 Targetted for a fooof 1.1 release. label Aug 28, 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! The only thing I could think of is if grid could be doc'ed in the plot_spectra docstring under **plot_kwargs or maybe the .plot method where plots are usually called from? Otherwise it may get hidden away / user may not know about since it's unlikely to call style_spectrum_plot directly.

@TomDonoghue
Copy link
Member Author

TomDonoghue commented Sep 5, 2023

Hey @ryanhammonds - yeh, I agree that a limitation / issue with our general strategy for applying plot style is a bit opaque, including for knowing what can be passed in. To try improve that, following your suggestion, I updated the docs in the spectra to mark the special-case of passing in grid, and also did a sweep through to try and better describe how plot_style works, and update the mentions of this across the board - can you take a quick check of the new updates and see if this seems better? I also extended the settable axis plot kwargs (following from NDSP - neurodsp-tools/neurodsp#319)

@fooof-tools fooof-tools deleted a comment from codecov-commenter Sep 13, 2023
@TomDonoghue
Copy link
Member Author

Going to merge this one in now to prepare a new release candidate!

@TomDonoghue TomDonoghue merged commit 4e673f2 into main Sep 13, 2023
@TomDonoghue TomDonoghue deleted the psdgrid branch September 13, 2023 12:39
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