Skip to content

By default, don't change the figure face/edgecolor on savefig(). #15111

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 4, 2020
Merged

By default, don't change the figure face/edgecolor on savefig(). #15111

merged 1 commit into from
May 4, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 23, 2019

This seems to repeatedly confuse users.

Closes #7619; fixes https://gitter.im/matplotlib/matplotlib?at=5d5faff768406739f96b2673; alternate for #10023.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@ImportanceOfBeingErnest
Copy link
Member

The dpi argument accepts "figure". Would it make sense to stay consistent with that and use "figure" also for the edge-, and facecolor?

In general this makes much more sense to me than #10023. I don't understand the exitstack thingy so I will leave this for others to review.

@anntzer
Copy link
Contributor Author

anntzer commented Aug 23, 2019

For colors, there's already places that accept "auto" and places that accept "inherit" (specifically, legends). I'd rather not add a third spelling for the same (general) idea. (In fact I plan to deprecate "inherit" in favor of "auto", but that's a separate issue.)

@ImportanceOfBeingErnest
Copy link
Member

"figure" is still also clearer about the origin of the value. "auto" could be anything.

@anntzer
Copy link
Contributor Author

anntzer commented Aug 24, 2019

So what do you think the "auto" cases of lines.markerfacecolor, lines.markeredgecolor, boxplot.flierprops.markerfacecolor, and axes.titlecolor should be named?

@tacaswell tacaswell added this to the v3.3.0 milestone Aug 24, 2019
@andrewzwicky andrewzwicky mentioned this pull request Jan 20, 2020
Closed
@anntzer anntzer mentioned this pull request May 4, 2020
Closed
@anntzer anntzer force-pushed the savefigcolorauto branch from 1552f80 to e425586 Compare May 4, 2020 09:06
@anntzer anntzer force-pushed the savefigcolorauto branch from e425586 to 57462b6 Compare May 4, 2020 10:35
efiring
efiring approved these changes May 4, 2020
View reviewed changes
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

Seems like a good idea: a move towards the goal of minimizing surprise.

@tacaswell
Copy link
Member

I see your your point @ImportanceOfBeingErnest , however I think that the ambiguity of "auto" is a benefit here because we can use the same spelling across a wide range of contexts.

@tacaswell tacaswell merged commit 7b7017e into matplotlib:master May 4, 2020
@anntzer anntzer deleted the savefigcolorauto branch May 4, 2020 19:22
@anntzer anntzer mentioned this pull request May 20, 2020
Closed
@QuLogic QuLogic mentioned this pull request Jun 3, 2021
Closed
This was referenced Apr 30, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efiring efiring efiring approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

Figure background colors
4 participants
@anntzer @ImportanceOfBeingErnest @tacaswell @efiring