Skip to content

[FIX]: Make inset axes transparent on savefig(..., transparent=True) #24816

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

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

chahak13
Copy link
Contributor

PR Summary

This is a continuation on #22816 to make sure savefig(..., transparent=True) handles nested figures and axes. Also updates the test figure to include doubly-nested cases to test against.

Closes issue #22674

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • [n/a] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • [n/a] New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

@image_comparison(['transparent_background'],
extensions=['png'], savefig_kwarg={'transparent': True},
remove_text=True)
def test_savefig_transparent():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anntzer one reason I did not implement the complete empty plot test mentioned in the previous PR was that that test would pass even if there is nothing being done while plotting?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but it's extremely unlikely that we'd ever introduce a bug such that ax.spines[:].set_visible(False); ax.set(xticks=[], yticks=[]) would actually disable all drawing.
If you want to feel extra safe I would still do a check_figures_equal where the reference figure has all axes manually made transparent with set_facecolor("none") calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay. Yeah, I'll do that then. Thanks.

Copy link
Contributor Author

@chahak13 chahak13 Dec 30, 2022

Choose a reason for hiding this comment

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

I had a quick question. Since this is related to a savefig parameter, I'll need to pass that to the savefig function inside the decorator. From what I can see, that's not possible right now with check_figures_equal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anntzer any suggestions on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like

@mpl.rc_context({"savefig.transparent": True})
@check_figures_equal(extensions=["png"])
def test_savefig_transparent(fig_test, fig_ref):
    # create two transparent subfigures with corresponding transparent inset
    # axes. the entire background of the image should be transparent.
    gs1 = fig_test.add_gridspec(3, 3, left=0.05, wspace=0.05)
    f1 = fig_test.add_subfigure(gs1[:, :])
    f2 = f1.add_subfigure(gs1[0, 0])

    ax12 = f2.add_subplot(gs1[:, :])

    ax1 = f1.add_subplot(gs1[:-1, :])
    iax1 = ax1.inset_axes([.1, .2, .3, .4])
    iax2 = iax1.inset_axes([.1, .2, .3, .4])

    ax2 = fig_test.add_subplot(gs1[-1, :-1])
    ax3 = fig_test.add_subplot(gs1[-1, -1])

    for ax in [ax12, ax1, iax1, iax2, ax2, ax3]:
        ax.set(xticks=[], yticks=[])
        ax.spines[:].set_visible(False)

    # and draw nothing on fig_ref.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not know about mpl.rc_context. This should work, thanks!

@chahak13
Copy link
Contributor Author

chahak13 commented Dec 26, 2022

PR cleanliness check is failing because of the test file being changed. I can squash all of it and force push but will that remove the original author's commit info since there's only one commit? I don't want to do that ideally.

@rcomer
Copy link
Member

rcomer commented Dec 26, 2022

@chahak13 you could add the original author as a co-author in the single commit:
https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

@QuLogic
Copy link
Member

QuLogic commented Jan 12, 2023

This appears to need a rebase to get the docs build working.

Co-authored-by: Shreeya Ramesh <shreeyar@umich.edu>
Co-authored-by: Chahak Mehta <chahakmehta013@gmail.com>
@chahak13 chahak13 force-pushed the transparent_inset_axes branch from b807359 to 0b3348a Compare January 26, 2023 19:18
@tacaswell tacaswell added this to the v3.8.0 milestone Jan 26, 2023
@ksunden ksunden merged commit 13a6ecf into matplotlib:main Jan 26, 2023
rcomer pushed a commit to rcomer/matplotlib that referenced this pull request Mar 21, 2023
Conflict was caused by the new test being right above the test from matplotlib#24816, which is not in the v3.7.x branch.

FIX: use locators in adjust_bbox
(cherry picked from commit 8ef978d)
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.

7 participants