Skip to content

Adding twin axes to an axis with GridHelperCurveLinear (from example) makes some ticklabels disappear #10748

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
Sup3rGeo opened this issue Mar 11, 2018 · 22 comments
Labels
status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. status: inactive Marked by the “Stale” Github Action topic: mpl_toolkit

Comments

@Sup3rGeo
Copy link

Bug report

Bug summary

Adding twin axes to another axis created with gridhelper GridHelperCurveLinear makes some of the ticklabels of the polar grid disappear.

Code for reproduction

Working fine:

import numpy as np
import matplotlib.pyplot as plt
from mpl_toolkits.axisartist import SubplotHost
from mpl_toolkits.axisartist.grid_helper_curvelinear import GridHelperCurveLinear
import mpl_toolkits.axisartist.angle_helper as angle_helper
from matplotlib.projections import PolarAxes
from matplotlib.transforms import Affine2D

class FormatterDMS(object):
    '''Transforms angle ticks to damping ratios'''
    def __call__(self,direction,factor,values):
        angles_deg = values/factor
        damping_ratios = np.cos((180-angles_deg)*np.pi/180)
        ret = ["%.2f"%val for val in damping_ratios]
        return ret

class ModifiedExtremeFinderCycle(angle_helper.ExtremeFinderCycle):
    '''Changed to allow only left hand-side polar grid'''
    def __call__(self, transform_xy, x1, y1, x2, y2):
        x_, y_ = np.linspace(x1, x2, self.nx), np.linspace(y1, y2, self.ny)
        x, y = np.meshgrid(x_, y_)
        lon, lat = transform_xy(np.ravel(x), np.ravel(y))

        with np.errstate(invalid='ignore'):
            if self.lon_cycle is not None:
                lon0 = np.nanmin(lon)
                lon -= 360. * ((lon - lon0) > 360.) # Changed from 180 to 360 to be able to span only 90-270 (left hand side)
            if self.lat_cycle is not None:
                lat0 = np.nanmin(lat)
                lat -= 360. * ((lat - lat0) > 360.) # Changed from 180 to 360 to be able to span only 90-270 (left hand side)

        lon_min, lon_max = np.nanmin(lon), np.nanmax(lon)
        lat_min, lat_max = np.nanmin(lat), np.nanmax(lat)

        lon_min, lon_max, lat_min, lat_max = \
            self._adjust_extremes(lon_min, lon_max, lat_min, lat_max)

        return lon_min, lon_max, lat_min, lat_max

def sgrid():
    # From matplotlib demos:
    # https://matplotlib.org/gallery/axisartist/demo_curvelinear_grid.html
    # https://matplotlib.org/gallery/axisartist/demo_floating_axis.html

    # PolarAxes.PolarTransform takes radian. However, we want our coordinate
    # system in degree
    tr = Affine2D().scale(np.pi/180., 1.) + PolarAxes.PolarTransform()
    # polar projection, which involves cycle, and also has limits in
    # its coordinates, needs a special method to find the extremes
    # (min, max of the coordinate within the view).

    # 20, 20 : number of sampling points along x, y direction
    sampling_points = 20
    extreme_finder = ModifiedExtremeFinderCycle(sampling_points, sampling_points,
                                                lon_cycle=360,
                                                lat_cycle=None,
                                                lon_minmax=(90,270),
                                                lat_minmax=(0, np.inf),)

    grid_locator1 = angle_helper.LocatorDMS(15)
    tick_formatter1 = FormatterDMS()
    grid_helper = GridHelperCurveLinear(tr,
                                        extreme_finder=extreme_finder,
                                        grid_locator1=grid_locator1,
                                        tick_formatter1=tick_formatter1
                                        )

    fig = plt.figure()
    ax = SubplotHost(fig, 1, 1, 1, grid_helper=grid_helper)

    # make ticklabels of right invisible, and top axis visible.
    visible = True
    ax.axis[:].major_ticklabels.set_visible(visible)
    ax.axis[:].major_ticks.set_visible(False)
    ax.axis[:].invert_ticklabel_direction()

    ax.axis["wnxneg"] = axis = ax.new_floating_axis(0, 180)
    axis.set_ticklabel_direction("-")
    axis.label.set_visible(False)

    # let left axis shows ticklabels for 1st coordinate (angle)
    ax.axis["left"].get_helper().nth_coord_ticks = 0
    ax.axis["right"].get_helper().nth_coord_ticks = 0
    ax.axis["left"].get_helper().nth_coord_ticks = 0
    ax.axis["bottom"].get_helper().nth_coord_ticks = 0

    fig.add_subplot(ax)
    ax.grid(True, zorder=0,linestyle='dotted')
    return ax, fig

ax, f = sgrid()
ax.plot([-5,5],[-5,5])

Results in:
good

Adding this however:

    par2 = ax.twin()
    new_fixed_axis = par2.get_grid_helper().new_fixed_axis
    par2.axis["left"] = new_fixed_axis(loc="left",
                                       axes=par2,
                                       offset=(0, 0))
    par2.axis["bottom"] = new_fixed_axis(loc="bottom",
                                       axes=par2,
                                       offset=(0, 0))

Results in lost ticklabels for the upper part:

lost information

Matplotlib version

  • Operating system: Windows 10
  • Matplotlib version: 2.1.2
  • Matplotlib backend (print(matplotlib.get_backend())): TkAgg
  • Python version: 3.6.4

Installed from pip.

@kmuehlbauer
Copy link
Contributor

@ImportanceOfBeingErnest Regarding your comment I'm trying to fix some things in mpl_toolkit.

I have found the source of this issue here, at least why the tickmarks etc are gone. The twin()-method makes the "top" and "right" axes invisible. This is actually the same behaviour as in twinx().

What needs decision is the wanted default behaviour. Should all possible axes be displayed by default? Sure the added image is not how we want to have it (all possible axes visible).

figure_1

@Sup3rGeo Any ideas how this should look like?

So, questions are, where to put the different axes and which should be made invisible. This decision also affects the remove method.

@kmuehlbauer
Copy link
Contributor

@smheidrich Sorry for disturbing, I'm trying to understand which axes need to be visible and which not. And for sure, the most logical solution. Looking at the commented code fragments before your changes in d268abf there has been quite some testing to probably find a nice solution. Do you remember why you've prefered this solution?

@smheidrich
Copy link
Contributor

smheidrich commented Oct 2, 2018

@kmuehlbauer The point behind my changes is summarized in the difference between the two images in this comment (which was also put into the tests as mpl_toolkits.tests.test_axes_grid1.test_twin_axes_empty_and_removed): previously, there was an (in my opinion, anyway) inconsistency between twinx/twiny and just twin, as seen in rows 3-5 of the first picture. My changes were only meant to make it consistent, but if this was a poor choice which messes up some other use cases then it should be either reverted altogether or made consistent in a different way.

By the way, there was another issue stemming from my changes that never got fixed, summarized here - but I guess that is a slightly different issue and this here relates to just twin and not set_visible? Regardless, that was another case where my changes didn't mesh well together with other parts of mpl_toolkits, because I really only tested the most simple case... very sorry about that. At this point it feels like it makes more sense to just revert my commit altogether (and then revert back associated example code, e.g. #5757 and #6836).

@ImportanceOfBeingErnest
Copy link
Member

I have to say I don't know what the expected outcome of the code here should be. I think the images in the comment in #4898 by @smheidrich are pretty convincing. So in this case I would think that due to the use of the twin method all axes visible is indeed expected? I suppose one can still set them invisible manually in case they are undesired? (If in doubt, I'd say removing something that is present is often easier than getting something to show that isn't.)
But my understanding of the axisartist module is pretty limited. That's why it is helpful if people who actually use this module work on fixing it. One main problem is the documentation in my eyes, since it currently does not allow to easily grasp the concepts.

@kmuehlbauer
Copy link
Contributor

@smheidrich Thanks for coming back to this, and for the pointers.

I'll have another look into this the next days adding some of my use cases here.

@ImportanceOfBeingErnest I adapted my use case from a matplotlib example. I'm not sure, if I used the API correctly but we will find out.

Thanks for responding, all.

@smheidrich
Copy link
Contributor

smheidrich commented Oct 2, 2018

It would still be consistent if twin* were changed to behave like twinx/twiny did prior to d268abf, i.e. if it added just the ticks and ticklabels, while keeping the axis line of the host. That would solve this issue, wouldn't it? Row 3 of the mentioned test comparison image would have to be updated, of course.

Are the plots which are generated for the gallery and docs part of CI by now, with image comparison tests? That would come in handy when changing anything in mpl_toolkits.

@ImportanceOfBeingErnest
Copy link
Member

Are the plots which are generated for the gallery and docs part of CI by now, with image comparison tests?

The documentation is tested in the sense that it generates successfully. If an example from the gallery was to error out, the test would fail. However there is no image comparisson. If an example simply produces some wrong image this is not caught. Of course one could simply copy the code from any example and create a test from it. In general, the number of image comparissons should not grow too large, and sometimes it would be enough to test for certain things like visibility or positions. Possibly, several examples can also be combined into a single image comparisson.

@smheidrich
Copy link
Contributor

I see, thanks!

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Oct 3, 2018

This is a stripped down version of my use case:

import matplotlib.pyplot as pl
import numpy as np

from matplotlib.projections import PolarAxes
from matplotlib.transforms import Affine2D
from mpl_toolkits.axisartist import (SubplotHost, GridHelperCurveLinear)
from mpl_toolkits.axisartist.grid_finder import FixedLocator, DictFormatter
import mpl_toolkits.axisartist.angle_helper as ah

# Transformation for polar data
# Set theta start to north
tr_rotate = Affine2D().translate(-90, 0)
# set theta running clockwise
tr_scale = Affine2D().scale(-np.pi / 180, 1)
# create transformation
tr = tr_rotate + tr_scale + PolarAxes.PolarTransform()

# build up curvelinear grid
extreme_finder = ah.ExtremeFinderCycle(20, 20,
                                       lon_cycle=360,
                                       lat_cycle=None,
                                       lon_minmax=(360, 0),
                                       lat_minmax=(0, np.inf),
                                       )

# locator and formatter for angle annotation
locs = [i for i in np.arange(0., 359., 10.)]
grid_locator1 = FixedLocator(locs)
tick_formatter1 = DictFormatter(dict([(i, r"${0:.0f}^\circ$".format(i))
                                      for i in locs]))

# grid_helper for curvelinear grid
grid_helper = GridHelperCurveLinear(tr,
                                    extreme_finder=extreme_finder,
                                    grid_locator1=grid_locator1,
                                    grid_locator2=None,
                                    tick_formatter1=tick_formatter1,
                                    tick_formatter2=None,
                                    )
# try to set nice locations for range gridlines
grid_helper.grid_finder.grid_locator2._nbins = 15.0
grid_helper.grid_finder.grid_locator2._steps = [0, 1, 1.5, 2,
                                                2.5,
                                                5,
                                                10]

fig = pl.figure(figsize=(6,5))

# generate curvelinear grid axis
cgax = SubplotHost(fig, 111, grid_helper=grid_helper)
fig.add_axes(cgax)

cgax.set_aspect('equal', adjustable='box')

# get twin axis for cartesian grid
caax = cgax.twin()

# make everything look right
# cgax.axis["top", "right"].set_visible(True)
# cgax.axis["top", "right"].major_ticklabels.set_visible(True)
# cgax.axis["left", "bottom"].major_ticks.set_visible(False)
# cgax.axis["left", "bottom"].major_ticklabels.set_visible(False)
# caax.axis["left", "bottom"].set_visible(True)
# caax.axis["left", "bottom"].major_ticklabels.set_visible(True)
# caax.axis["top", "right"].set_visible(False)

# show curvelinear and cartesian grids
cgax.grid(True)
caax.grid(True)

t = pl.title('Simple Curvelinear Grid')
t.set_y(1.05)

pl.show()

Retested with d5a4eda, updated code

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Oct 3, 2018

Status Quo, created with d5a4eda
status_quo

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Oct 3, 2018

Wanted, created with d5a4eda
wanted

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Oct 3, 2018

Just remove the comments below the line containing # make everything look right to get the wanted output.

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Oct 3, 2018

The current behaviour makes top and right of cgax invisible, remaining left and bottom. But cgax.twin() is adding to left and bottom too, which leads to this overplotting. To make everything look nice, I had to make top and right of cgax visible again as well as the ticklabels. Also had to make left and bottom of caax visible and top and right invisible.

This is all true for the current configuration (xlim/ylim: 0,1). But think of (xlim/ylim: -1,0), Then the different axes need to be on the opposite of each other (at least the axes with the angles, see image below, created with d5a4eda) .

So, currently I'm a bit at loss, if there is single fitting solution at all.
broken

@kmuehlbauer
Copy link
Contributor

Should it be that easy now? OK, in twin()-method the following is done:

self.axis["top", "right"].set_visible(False)
ax2.axis["top", "right"].set_visible(True)
ax2.axis["left", "bottom"].set_visible(False)

So the top and right of the twin-axes are set visible. But this does not activate the ticks etc.
So if I take my example from above and just do:

# make everything look right
caax.axis["top", "right"].toggle(all=True)

then I get an image with all wanted axes (see below, created with d5a4eda). We see that the placement of the bottom axes is not good. But we can't do much about it. Now the user has to switch axes to his liking.

better

@kmuehlbauer
Copy link
Contributor

In the light of these examples, it seems that the only problem with the current code is, that the ticks etc are not visible. My question is: Is this intended behaviour?

@kmuehlbauer
Copy link
Contributor

@smheidrich @ImportanceOfBeingErnest Can someone please sanity check?

The problem of the original poster @Sup3rGeo is that his angular (?) axis spans over three axes (top, left, bottom). left and bottom of the original axis remain untouched, but top and right are made unvisible within twin().

To solve this just one line has to be added restoring the original top axis. (tested with d5a4eda)

ax.axis["top"].set_visible(True)

wanted_orig

@Sup3rGeo
Copy link
Author

Sup3rGeo commented Oct 3, 2018

I am actually quite beginner with matplotlib and it has been a while since I opened this one, so I am not totally sharp.

But I think I agree with @ImportanceOfBeingErnest:

So in this case I would think that due to the use of the twin method all axes visible is indeed expected? I suppose one can still set them invisible manually in case they are undesired? (If in doubt, I'd say removing something that is present is often easier than getting something to show that isn't.)
(...) One main problem is the documentation in my eyes, since it currently does not allow to easily grasp the concepts.

In general I would not expect anything on the host axis to be hidden when adding a twin, it was a bit confusing. Then when adjusting the axis, if there is no one-size-fits-all, the possibilities should be at least well documented.

@kmuehlbauer
Copy link
Contributor

@Sup3rGeo Thanks for responding. I would also agree, that it's a bit confusing if parts of the original host axis are silently hidden. I searched the current examples/demos but couldn't find anything related to this specific behaviour. Why not add nice example showing how to handle this specific use case?

Also the twin()-function docstring needs an update, it is the same as twinx() currently.

Other opinions?

@smheidrich
Copy link
Contributor

smheidrich commented Oct 5, 2018

Just so I understand: Does the issue of "angle" ticklabels being drawn on top of each other in the bottom left corner of #10748 (comment) have anything to do with this twin issue? At least in my testing, this happens even if I remove the twin call altogether, but I don't really get what's going on there anyway.

If it is a separate issue, the only issue here is that nothing on the host axes should be hidden when using any of the twin* methods, right? I think I'd agree that this would be better default behavior. It would also simplify the _remove_methods to just calls of self.parasites.remove(h).

As I wrote in #10748 (comment), this basically means changing twin* to behave like twinx/twiny did before d268abf, i.e. you'd call ax["right"].line.set_visible(False) in twinx to hide the parasite axis lines instead of the ones of the host, and analogously for twiny and twin. In contrast to pre- d268abf behavior, it would still be consistent between all the twin* methods.

I also agree that the twin docstring should definitely be fixed - that seems like a fairly uncontroversial change :)

@kmuehlbauer
Copy link
Contributor

Just so I understand: Does the issue of "angle" ticklabels being drawn on top of each other in the bottom left corner of #10748 (comment) have anything to do with this twin issue? At least in my testing, this happens even if I remove the twin call altogether, but I don't really get what's going on there anyway.

@smheidrich Correct, this particular problem has nothing to do with this issue, hence I wrote "But we can't do much about it."

If it is a separate issue, the only issue here is that nothing on the host axes should be hidden when using any of the twin* methods, right? I think I'd agree that this would be better default behavior. It would also simplify the _remove_methods to just calls of self.parasites.remove(h).

This sounds reasonable to me. It does not hide anything on the host axes and simplifies code.

I also agree that the twin docstring should definitely be fixed - that seems like a fairly uncontroversial change :)

For sure! 😁

@smheidrich
Copy link
Contributor

smheidrich commented Oct 8, 2018

All right then, should I submit a PR or is anyone else already working on it? Or is there something else to discuss first?

smheidrich added a commit to smheidrich/matplotlib that referenced this issue Dec 30, 2018
PR matplotlib#4896 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4896 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this issue Jan 3, 2019
PR matplotlib#4896 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4896 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this issue Jan 3, 2019
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this issue Jan 3, 2019
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this issue Jan 3, 2019
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this issue Jan 4, 2019
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
@github-actions
Copy link

github-actions bot commented May 2, 2023

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label May 2, 2023
@github-actions github-actions bot added the status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. label Jun 1, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. status: inactive Marked by the “Stale” Github Action topic: mpl_toolkit
Projects
None yet
Development

No branches or pull requests

5 participants