-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate {ContourSet,Quiver}.ax in favor of .axes. #16058
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
Conversation
The use of attributes
Step 1: Current stateUsages of
Usages of
Step 2: Preferred solutionPro
Pro 'self.axes`:
IMHO, there's no clear winner. I slightly lean towards keeping |
Just to be clear, it's a lot more classes, in practice (not just 13 vs 7 like the count above suggests): all Artists have .axes, so that's also Line2D, all the Collections, all the Patches, etc... which are also much more commonly used than any of the classes that use .ax. And as a consequence, other classes (e.g. Colorbar, as noted in #15986 (comment), but also Animation) also typically assume that they can access parent axes of other objects (which are typically artists, but not always) with |
As the most likely guilty party in establishing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also fine with switching to axes
.
@tacaswell since this change may possibly affect a number of users, IMHO you should also give your consent.
For consistency with other artists. In particular, Colorbar.remove assumes that the `.axes` attribute exists. Also a smattering of cleanups.
rebased |
As we don't have a good sense of how big of an impact this actually has I am in favor of merging this and seeing what push back we get in the rc / 3.3.0. We should definitly have the |
For consistency with other artists. In particular, Colorbar.remove
assumes that the
.axes
attribute exists.Also a smattering of cleanups.
Edit: also do the same for ContourSet, per #12443 (comment).
PR Summary
PR Checklist