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] - Plotting updates #343

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

[ENH] - Plotting updates #343

wants to merge 12 commits into from

Conversation

TomDonoghue
Copy link
Member

@TomDonoghue TomDonoghue commented Nov 30, 2024

This PR updates and extends some plotting code. It adds to neurodsp some code I developed in the AperiodicHistory project, which I think are general enough to be added her.

Main updates:

  • Create a new prepare_multi_plot which consolidates the same process we had in multiple places for supporting plots that may (or may not) have multiple inputs (e.g. plotting multiple spectra on the same plot)
  • Adds a new plot_autocorr plot
  • Adds a new plot_spectra_3D plot

@neurodsp-tools neurodsp-tools deleted a comment from codecov bot Nov 30, 2024
@neurodsp-tools neurodsp-tools deleted a comment from codecov bot Nov 30, 2024
@TomDonoghue TomDonoghue changed the title [WIP] - Plotting updates [ENH] - Plotting updates Dec 3, 2024
@neurodsp-tools neurodsp-tools deleted a comment from codecov bot Dec 3, 2024
@neurodsp-tools neurodsp-tools deleted a comment from codecov bot Dec 5, 2024
Copy link
Member

@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!


@savefig
@style_plot
def plot_autocorr(timepoints, autocorrs, labels=None, colors=None, ax=None, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Only thought here is whether to include it in aperiodic.py or somewhere else, since acf isn't restricted to aperiodic signals.

... 'sim_bursty_oscillation' : {'freq': 10}})
>>> freqs1, powers1 = compute_spectrum(sig1, fs=500)
>>> freqs2, powers2 = compute_spectrum(sig2, fs=500)
>>> plot_spectra_3D([freqs1, freqs2], [powers1, powers2])
Copy link
Member

Choose a reason for hiding this comment

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

I noticed the z-axis label is cutoff when I copy/paste the example into a notebook. I tried plt.tight_layout() but it gave an error about the size of the fig not having large enough margins. Maybe a solution would be to add an ax or fig kwarg


@savefig
@style_plot
def plot_spectra_3D(freqs, powers, log_freqs=False, log_powers=True,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be added to __init__.py?

else:
labels = repeat(labels)

colors = repeat(colors) if not isinstance(colors, list) else cycle(colors)
Copy link
Member

Choose a reason for hiding this comment

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

The docstring for colors and labels says it accepts iterables, but this may break with tuple/array.

@neurodsp-tools neurodsp-tools deleted a comment from codecov bot Dec 18, 2024
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

Successfully merging this pull request may close these issues.

2 participants