Skip to content

FIX: allow add option for Axes3D(fig) #19413

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

Closed
wants to merge 3 commits into from

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Feb 1, 2021

PR Summary

Closes #18939

We changed the logic to deal with Axes3d(fig) to not add the new axes to fig in #18564, but of course that broke some people who were working off older examples.

This PR pops up a DeprecationWarning if folks use the old call, however, if they do the somewhat awkward:

ax = Axes3d(fig)
fig.add_axes(ax)

they will also get a Deprecation Warning, which they can suppress with

ax = Axes3d(fig, add=False)
fig.add_axes(ax)

However, the deprecation warning just suggests that they add the subplot the idiomatic way:

ax = fig.add_subplot(projection='3d')

I'm not a super fan of this proposal - it pops up a warning for a valid if rare use case, but its better than mysteriously not adding the Axes3d object. and we can rip it all out in a cycle or two.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzer anntzer added this to the v3.4.0 milestone Feb 1, 2021
@jklymak
Copy link
Member Author

jklymak commented Feb 1, 2021

OK, and interestingly Axes3D(fig), fig.add_axes(ax) is not the same as fig.add_subplot(projection='3d'). So I'm not sure if there is a way to get this to work without asking for a kwarg.

Old

Old

New

New

@jklymak jklymak added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Feb 1, 2021
@jklymak
Copy link
Member Author

jklymak commented Feb 1, 2021

(BTW Release Critical as the breaking change hasn't been released yet)

@WeatherGod
Copy link
Member

Just a historical note... originally, one created Axes3D by doing fig.add_axes(ax). This usually meant that the Axes3D object was not a "subplot" axes, and therefore, it did not get certain margins set. That is why the sizes of the plots are different between the two methods.

@jklymak
Copy link
Member Author

jklymak commented Feb 1, 2021

Yeah given the differences I'm still a little torn about how to do this.

Sticking add=False through our code isn't particularly nice.

Adding the axes twice is unacceptable.

Emitting a deprecation warning for something we think is acceptable is non optimal as well.

I guess another option is to have add_axes check if the same axes object is already on the stack and not add it again.

@jklymak
Copy link
Member Author

jklymak commented Feb 7, 2021

Waiting for #19438

@tacaswell
Copy link
Member

Rather that touching the base classes __init__ can we spcial-case 3D in _process_projection_requirements so that only the Axes3D instances will see the extra kwarg?

@jklymak
Copy link
Member Author

jklymak commented Feb 10, 2021

Possibly? If you are in a groove and thinking about this I wouldn't be upset with a competing PR.

@jklymak
Copy link
Member Author

jklymak commented Feb 17, 2021

Closing in lieu of #19496

@jklymak jklymak closed this Feb 17, 2021
@jklymak jklymak deleted the fix-3daxes branch February 17, 2021 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. status: duplicate status: waiting for other PR topic: mplot3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn re: Axes3D constructor behavior change in mpl3.4
6 participants