8000 Deprecate add_subplot(<no positional args>) silently doing nothing. by anntzer · Pull Request #13127 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Deprecate add_subplot(<no positional args>) silently doing nothing. #13127

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
Jan 15, 2019
Merged

Deprecate add_subplot(<no positional args>) silently doing nothing. #13127

merged 1 commit into from
Jan 15, 2019

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Jan 7, 2019

To avoid head-scratching when add_subplot(projection="3d") does
nothing (the correct call is add_subplot(111, projection="3d")).

Also a poll: after the deprecation period, should add_subplot(<no positional args>)

  • raise an exception?
  • behave as add_subplot(111, **kwargs) (see the mplot3d case above for a reasonable use case)?

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

@anntzer anntzer added this to the v3.1 milestone Jan 7, 2019
@anntzer anntzer force-pushed the add_subplot-no-args branch from b73a94a to f9029d6 Compare January 7, 2019 10:14
@jklymak
Copy link
Member
jklymak commented Jan 7, 2019

Looks like it needs a test.

@anntzer
Copy link
Contributor Author
anntzer commented Jan 7, 2019

Seems like a bit overkill right now; if we do make add_subplots() behave like add_subplots(111) then I'll add a test at that time...

@jklymak
Copy link
Member
jklymak commented Jan 7, 2019 •
edited
Loading

POLL: add_subplot() adds a plot... thumbs down if it should error...

@tacaswell
Copy link
Member
  • add_subplot() should add an axes to match the behavior of plt.subplots()
  • add_subplot() should raise because it has never done anything before.

The projection='3d' case is pretty persuasive for the first one.

@anntzer
Copy link
Contributor Author
anntzer commented Jan 8, 2019 •
edited
Loading

Do we even want a deprecation period here, or should we just switch directly to adding the 111 plot?
POLL: Upvote for switching immediately, downvote for a deprecation period.

@tacaswell
Copy link
Member

What is the use-case story for someone intentionally passing nothing into add_subplot and relying on it doing nothing? The best I can come up with is some elaborate custom GUI embedding where they are relying on us to do the "no user input -> no action" logic rather than owning it, but that is a stretch.

@anntzer
Copy link
Contributor Author
anntzer commented Jan 8, 2019

The same question applies to many other changes I've proposed over the years, but I've learnt better...

@tacaswell
Copy link
Member

This code appears to go back to 18dca66 .

add_axes has the same behavior, we should change it in both places.

@anntzer I do feel your frustration, but telling apart "there is no use case, it is like this by coincidence" from "this is critical to a use-case that I am not familiar with" from "this used to be part of an important use-case that no longer exists" is subtle (because it involves proving a negative and / or knowing what you don't know).

@anntzer
Copy link
Contributor Author
anntzer commented Jan 13, 2019

@jklymak why the needs-revision tag? Did we decide to just skip the deprecation?

@jklymak
Copy link
Member
jklymak commented Jan 13, 2019

Yeah I think so, right? As in no one could think of a case where the do nothing behaviour would be desirable.
8000

@jklymak
Copy link
Member
jklymak commented Jan 13, 2019

I guess add_axes should also be checked out as well?

@anntzer
Copy link
Contributor Author
anntzer commented Jan 13, 2019

I don't think there's a sane default rectangle for add_axes?

@anntzer
Deprecate add_subplot(<no positional args>) silently doing nothing.
9eb7f87 
To avoid head-scratching when `add_subplot(projection="3d")` does
nothing (the correct call is `add_subplot(111, projection="3d")`).
@anntzer anntzer force-pushed the add_subplot-no-args branch from f9029d6 to 9eb7f87 Compare January 13, 2019 21:56
@anntzer
Copy link
Contributor Author
anntzer commented Jan 13, 2019

Changed the behavior of add_subplot as suggested.

@jklymak
Copy link
Member
jklymak commented Jan 15, 2019

I'm going to merge on the basis of the polls above, despite the fact @dstansby's approval somewhat predated those changes.

@jklymak jklymak merged commit 5ad49b4 into matplotlib:master Jan 15, 2019
@anntzer anntzer deleted the add_subplot-no-args branch January 15, 2019 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jklymak jklymak jklymak approved these changes

@dstansby dstansby dstansby approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@anntzer @jklymak @tacaswell @dstansby
0