8000 [MAINT] savefig only takes one args by NelleV · Pull Request #9090 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

[MAINT] savefig only takes one args #9090

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
Aug 24, 2017
Merged

[MAINT] savefig only takes one args #9090

merged 1 commit into from
Aug 24, 2017

Conversation

NelleV
Copy link
Member
@NelleV NelleV commented Aug 24, 2017

Making this explicit improves error message when user provides
two args to the function.

  • Has Pytest style unit tests
  • Code is PEP 8 compliant

@NelleV
[MAINT] savefig only takes one args
aee94c9 
Making this explicit improves error message when user provides
two args to the function.
@NelleV
Copy link
Member Author
NelleV commented Aug 24, 2017 •
edited
Loading

Running into a github glitch. Supersedes #9089

@NelleV NelleV closed this Aug 24, 2017
@NelleV NelleV reopened this Aug 24, 2017
@dopplershift
Copy link
Contributor

What I was about to say was:

So technically this is a backwards-compatibility break for code that does:

fig.savefig('foo.png', 300, 'blue')

where the last two would be dpi and face color. Not saying that's good code, but there's no ambiguity there from the user who wrote it (and who presumably doesn't believe in keyword arguments).

Now that I read it more closely, I see that's not the case, which is really confusing. So 👍 to cleaning that up.

@fariza fariza merged commit a226171 into matplotlib:master Aug 24, 2017
@QuLogic QuLogic added this to the 2.1 (next point release) milestone Aug 24, 2017
@tacaswell
Copy link
Member

Thanks @NelleV !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dopplershift dopplershift dopplershift approved these changes

@fariza fariza fariza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.
5 participants
@NelleV @dopplershift @tacaswell @fariza @QuLogic
0