Skip to content
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

DOC: missing second y-axis in `demo_parasite_axes2` #6836

Closed
afvincent opened this issue Jul 26, 2016 · 12 comments

Comments

Projects
None yet
6 participants
@afvincent
Copy link
Contributor

commented Jul 26, 2016

It looks like the example demo_parasite_axes2 lost its “Temperature” second y-axis in the devdocs gallery. See the non-devodcs version for a comparison.

For those who do not like to click, here are the two PNGs with default dpi:

  • devdocs version
    devdocs_demo_parasite_axes2_00_00
  • non devdocs version
    non-devdocs_demo_parasite_axes2_00_00

NB: a diff between the code source of each example results in an empty file.

Edit: “twin y-axis” ← “second y-axis”, which should be more correct.

@afvincent

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2016

Besides, I do not use that much the mpl_toolkits but is it normal that mpl_toolkits.axisartist uses “inward” tick marks? (I mean, while mpl defaults are now to use “outwards” tick marks)

@afvincent afvincent changed the title DOC: missing twin-y axis in `demo_parasite_axes2` DOC: missing second y axis in `demo_parasite_axes2` Jul 26, 2016

@afvincent afvincent changed the title DOC: missing second y axis in `demo_parasite_axes2` DOC: missing second y-axis in `demo_parasite_axes2` Jul 26, 2016

@jenshnielsen jenshnielsen added this to the 2.0 (style change major release) milestone Jul 26, 2016

@efiring

This comment has been minimized.

Copy link
Member

commented Jul 26, 2016

It works correctly on v2.x...
Tick direction: good point. If you are looking for a challenge, see if you can figure out what is determining the tick direction and other properties...

@afvincent afvincent self-assigned this Jul 26, 2016

@afvincent

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2016

@efiring What version of mpl 2.x did you test? On my computer, with mpl 2.0.0b3.post1859+g80a3f3e (retrieved a few hours ago from GitHub), I get the same picture as in the devdocs gallery, with the second y-axis missing.

I will try to have a look at it, as well as at the tick direction and Cie.

@efiring

This comment has been minimized.

Copy link
Member

commented Jul 26, 2016

In [5]: matplotlib.version
'2.0.0b3+23.g18ffa3e'

@QuLogic QuLogic modified the milestones: 2.1 (next point release), 2.0 (style change major release) Jul 27, 2016

@QuLogic

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

This is broken on master, but not v2.x. Bisect points to d268abf:

d268abf8fd9c41fdc37b9bfeb8b1d576b4b288d3 is the first bad commit
commit d268abf8fd9c41fdc37b9bfeb8b1d576b4b288d3
Author: productivememberofsociety666 <productivememberofsociety666@sol.fr.am>
Date:   Fri Aug 14 19:54:02 2015 +0200

    Changed twin* functions so the corresponding host axes are invisible; updated _remove_method.

:040000 040000 27e94ea209af1f49c24fe81628ad97d51f772a11 c1bbecea359621967069ba84b1e81a1ee6ffd2a5 M  lib
@afvincent

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2016

@QuLogic Thank you for figuring out the bisecting point!

I think I may have found the reason why the 2nd y-axis is not visible after d268abf. For example in twinx from axes_grid1.parasite_axes.py, one has (the comments are my own understanding of the code):

  • before:
# 1/ Deactivate the right side of the parent axis, but its line.
self.axis["right"].toggle(all=False)
self.axis["right"].line.set_visible(True)

# 2/ Hide all the sides of the parasite axe but the right side.
ax2.axis["right"].set_visible(True)
ax2.axis["left","top", "bottom"].toggle(all=False)
ax2.axis["left","top", "bottom"].line.set_visible(False)

# 3/ Setup the right side of the parasite ax.
ax2.axis["right"].toggle(all=True)  # Do not forget to activate tick & labels!
ax2.axis["right"].line.set_visible(False)  # the one from parent ax is visible
  • after:
# 1/ Simply make the right side of the parent ax not visible.
self.axis["right"].set_visible(False)

# 2/ Make all sides of the parasite ax not visible but the right one.
ax2.axis["right"].set_visible(True)
ax2.axis["left", "top", "bottom"].set_visible(False)

# 3/ Setup the right side of the parasite ax.
# Empty!

and if I add ax2.axis["right"].toggle(all=True) in the 3/, then the demo seems to be OK.

I did not have time to look carefully to the related PR (#4898) yet: I cannot tell for the moment that this is not going against the aim of #4898. I will have a better look at this tomorrow or this weekend.

@smheidrich

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2016

@afvincent Everything that #4898 was meant to achieve has been put into the mpl_toolkits.tests.test_axes_grid1.test_twin_axes_empty_and_removed test and is illustrated by the difference between the pictures in this comment. Your proposed change doesn't seem to make the test fail, so it should be fine.

Although I don't really get why it doesn't work the way it is. I think it has something to do with the AA.Axes class (which I didn't really consider when writing #4898 - a bad idea given that it's the main selling point of the axes_grid toolkit, sorry about that 😟): if I remove everything that has anything to do with that class, I at least get both twin axes visible, although of course it looks like garbage because I can't have the offset without AA.Axes.

Anyway, I think your suggestion should fix it.

@afvincent

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2016

@smheidrich Thank you for the explanations as well as for the experiments :)!

I have played a bit with the test you added in #4898 (doing more or less the same kind of things as you did in gist) and it seemed to me too that the problem has to do with AA.Axes. I will try to investigate this in the next few days, to see if there is a deeper ground to the issue, that could also explain the minor issue with tick directions & Cie.

smheidrich added a commit to smheidrich/matplotlib that referenced this issue Aug 1, 2016

Potential fix for matplotlib#6836.
Makes mpl_toolkits.axisartist.AxisArtist.set_visible behave like
mpl_toolkits.axes_grid1.mpl_axes.SimpleAxisArtist, i.e. it now toggles
all child elements as well as setting what is normally called
"visibility" in matplotlib.
@smheidrich

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2016

All right, so to me it seems this comes down to the difference between axis artists that are used by the default axes vs mpl_toolkits.axisartist.Axes, specifically what exactly their set_visible methods do:

So that explains why my code worked for default axes without the explicit toggle call, but not for the axisartist axes.

I tested this by making axisartist.AxisArtist.set_visible behave like axes_grid1.mpl_axes.SimpleAxisArtist.set_visible and then demo_parasite_axes2.py works again, although knowing my luck this probably breaks something else. :P

Also I think the better solution might be to do the exact opposite and make axes_grid1.mpl_axes.SimpleAxisArtist.set_visible behave like axisartist.AxisArist.set_visible? Or maybe leave both methods as they are? In either case you would then have to add the toggle code to twin* like @afvincent originally suggested.

But in general one problem with the axes grid toolkit it's that it's so damn inconsistent, so perhaps having at least set_visible behave the same might be nice.

@tacaswell

This comment has been minimized.

Copy link
Member

commented Aug 2, 2016

👍 making set_visible apply to the children and making AA more consistent.

@smheidrich

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2016

@tacaswell It might be better to make it consistent the other way round, i.e. by making set_visible not toggle any children ever. I guess the way to think about it is that toggle changes the properties of the axisartist. Then set_visible just determines whether the axisartist as a whole is shown, whatever its properties (with/without ticks or ticklabels) might be. In more practical terms, I think it's plausible that some people do use axisartists with e.g. their ticklables turned off as a conscious choice, and they don't expect set_visible to override this choice. I'm not sure though.

smheidrich added a commit to smheidrich/matplotlib that referenced this issue Nov 12, 2016

smheidrich added a commit to smheidrich/matplotlib that referenced this issue Nov 12, 2016

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Oct 3, 2017

@QuLogic

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

Was fixed by above PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.