Skip to content

Don't draw axis (spines, ticks, labels) twice when using parasite axes. - #11688

Merged
tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:axislines-draw-twice
Sep 23, 2018
Merged

tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:axislines-draw-twice

Conversation

@anntzer

@anntzer anntzer commented Jul 18, 2018

Copy link
Copy Markdown
Contributor

We can just defer to the superclass' draw (axes._base.Axes.draw)
which will correctly draw everything in get_children();
axislines.Axes.get_children already adds the axislines and gridlines to
the superclass' get_children().

Likewise, get_tightbbox() doesn't need to be overridden because the
superclass' get_tightbbox by default uses
get_default_bbox_extra_artists, which fetches all visible children.
(This point is not needed for the bugfix, but we may as well...)

Don't bother using remove_text in the new figures, because that doesn't
remove the labels anyways (and the tests are also about label placement).

Issue pointed out by @tacaswell in #11602 (comment) and @ImportanceOfBeingErnest in #11602 (comment).

I feel like I should charge a quarter for every bugfix that only involves deleting code :)

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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

@ImportanceOfBeingErnest

Copy link
Copy Markdown
Member

👍 🎉

Before:

image

After:

image

@ImportanceOfBeingErnest

Copy link
Copy Markdown
Member

As this affects every axes drawn with the toolkits, I would think this is significant enough to get a what's new entry (there are no bug-fixed entries, right?) to tell people that those axes now look as great as the usual matplotlib axes and are hence even usable for publication quality figures.

Is it too late to get this into 3.0?

@anntzer
anntzer force-pushed the axislines-draw-twice branch from b0ced54 to 8e88094 Compare July 18, 2018 12:18
@anntzer

anntzer commented Jul 18, 2018

Copy link
Copy Markdown
Contributor Author

Added a whatsnew. I don't particularly care re: 3.0 or not.

We can just defer to the superclass' draw (axes._base.Axes.draw)
which will correctly draw everything in get_children();
axislines.Axes.get_children already adds the axislines and gridlines to
the superclass' get_children().

Likewise, get_tightbbox() doesn't need to be overridden because the
superclass' get_tightbbox by default uses
get_default_bbox_extra_artists, which fetches all visible children.
(This point is not needed for the bugfix, but we may as well...)

Don't bother using remove_text in the new figures, because that doesn't
remove the labels anyways (and the tests are also about label placement).
@ImportanceOfBeingErnest ImportanceOfBeingErnest added this to the v3.0.x milestone Sep 22, 2018
@tacaswell
tacaswell merged commit e771c1b into matplotlib:master Sep 23, 2018
return axis

def draw(self, renderer, inframe=False):
if not self._axisline_on:

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.

This is correctly handled in get_children

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 23, 2018
dstansby added a commit that referenced this pull request Sep 23, 2018
…688-on-v3.0.x

Backport PR #11688 on branch v3.0.x (Don't draw axis (spines, ticks, labels) twice when using parasite axes.)
@anntzer
anntzer deleted the axislines-draw-twice branch September 23, 2018 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants