Skip to content

Deprecate {ContourSet,Quiver}.ax in favor of .axes. - #16058

Merged
tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:quiverax
Apr 13, 2020
Merged

tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:quiverax

Conversation

@anntzer

@anntzer anntzer commented Dec 31, 2019 •

Copy link
Copy Markdown
Contributor

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

  • 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

@timhoffm

timhoffm commented Jan 1, 2020 •

Copy link
Copy Markdown
Member

The use of attributes .ax vs .axes is a mess. Instead of fixing single issues like here or in ContourSets in #12443, we need to

  1. Evaluate the current state
  2. Decide which one is the preferred solution.
  3. Consistently change it everywhere in the code base.

Step 1: Current state

Usages of self.ax (searched for self.ax = ):

  • Quiver
  • ColorbarBase
  • ContourSet
  • Sankey
  • AxesWidget
  • ToolHandles
  • axes_grid1.ColorbarBase

Usages of self.axes (searched for self.axes = ):

  • AxisArtist
  • Artist
  • _AxesBase (which is an Artist)
  • axis_grid1.Axes
  • Tick
  • Axis
  • _ImageBase
  • Legend
  • VertexSelector
  • _ThetaShift
  • Spine
  • MultiCursor
  • GridHelperRectlinear

Step 2: Preferred solution

Pro self.ax:

  • The canonical variable name is ax (fig, ax = plt.subplots()).
    • It's easy to guess/remember if you are familiar with matplotlib conventions.
    • That choice was deliberate to prevent plural naming issues when collections containing Axes.

Pro 'self.axes`:

  • More classes currently use this variant. But I'm not clear for which classes this attribute is accessed by users; so not sure deprecating ax would have the smaller impact on users.
  • The name is more readable for the uninformed user (but then again does he know what an axes is?).

IMHO, there's no clear winner. I slightly lean towards keeping self.ax. But this is really up for discussion.

@timhoffm timhoffm added API: changes Changes to the public API, typically requiring deprecation. API: consistency Consistency of the matplotlib API, including naming, behavior, defaults, … status: needs comment/discussion needs consensus on next step labels Jan 1, 2020
@anntzer

anntzer commented Jan 1, 2020 •

Copy link
Copy Markdown
Contributor Author

More classes currently use this variant.

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 .axes.

@anntzer anntzer changed the title Deprecate Quiver.ax in favor of Quiver.axes. Deprecate {ContourSet,Quiver}.ax in favor of .axes. Mar 25, 2020
@efiring

efiring commented Apr 7, 2020

Copy link
Copy Markdown
Member

As the most likely guilty party in establishing the ax attributes, I'm OK with switching to axes. It will be interesting to see how much external code this ends up affecting.

@timhoffm timhoffm left a comment

Copy link
Copy Markdown
Member

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.
@anntzer

anntzer commented Apr 12, 2020

Copy link
Copy Markdown
Contributor Author

rebased

@tacaswell tacaswell added this to the v3.3.0 milestone Apr 13, 2020
@tacaswell

Copy link
Copy Markdown
Member

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 .axes alias to make the API more uniform and it would be better to have only one way to do this, but if we get too many complaints we should be ready to leave .ax permanently.

@tacaswell
tacaswell merged commit 0824d00 into matplotlib:master Apr 13, 2020
@anntzer
anntzer deleted the quiverax branch April 13, 2020 19:50
@QuLogic QuLogic removed the status: needs comment/discussion needs consensus on next step label Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: changes Changes to the public API, typically requiring deprecation. API: consistency Consistency of the matplotlib API, including naming, behavior, defaults, …

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants