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

enable passing of external Axes to plot on #802

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

egpbos
Copy link
Member

@egpbos egpbos commented Jun 18, 2024

Adds to the plot_image and plot_tabular functions an optional argument ax: plt.Axes. When given, this is the Axes that will be used to plot on and the internal plt.subplots call is skipped. This is useful for using these functions in custom multi-panel plots (we want to use this in a paper).

We actually only need this for the plot_image function, but I added it to plot_tabular as a bonus ;) The plot_timeseries function is too complicated, I don't have time to add it there right now, but the principle will be similar (except now the ax has to be passed through three layers of helper functions).

@egpbos egpbos requested a review from stefsmeets June 18, 2024 10:53
@egpbos
Copy link
Member Author

egpbos commented Jun 18, 2024

P.S.: "we" here is me and @cwmeijer :)

@egpbos
Copy link
Member Author

egpbos commented Jun 18, 2024

Whoops, forgot to rebase on main, done now.

Copy link
Contributor

@stefsmeets stefsmeets 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 to me! 🚀🚀

Minor nitpick would be to also add types to the heatmap plot while you are at it 😅

@egpbos
Copy link
Member Author

egpbos commented Jun 19, 2024

Your nitpick is my command ;)

I did not hint it completely. The matplotlib stuff is just too complex and bloaty and increases maintenance cost. I think the reference in the docstring to go look at mpl.Axes.imshow is best for those options.

@egpbos
Copy link
Member Author

egpbos commented Jun 19, 2024

The CI failures are HTTP timeouts unrelated to this PR.

Copy link
Contributor

@cwmeijer cwmeijer left a comment

Choose a reason for hiding this comment

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

Top! Could you first accept my two other suggestions (same as in the other file)? Thanks for this addition!

dianna/visualization/tabular.py Show resolved Hide resolved
dianna/visualization/tabular.py Show resolved Hide resolved
@egpbos
Copy link
Member Author

egpbos commented Jun 19, 2024

Will you merge when you think it's done?

Adds to the plot_image and plot_tabular functions an optional argument ax: plt.Axes. When given, this is the Axes that will be used to plot on and the internal plt.subplots call is skipped. This is useful for using these functions in custom multi-panel plots (we want to use this in a paper).
@egpbos egpbos merged commit 14355a1 into main Jun 26, 2024
17 checks passed
@egpbos egpbos deleted the external_Axes_passing branch June 30, 2024 18:28
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.

None yet

3 participants