Fixed ZoomPanBase to work with log plots

[jrmlhermitte]

Tested this code in a matplotlib 1.4.3 environment and current branch. Fixed ZoomPanBase to work in non-linear plots by changing the scale in the axes coordinates and using the respective tranformation functions: transScale and transLimits transforms.

See tutorial on axes transformations for more details:
http://matplotlib.org/users/transforms_tutorial.html

Note: It did not work in my version 1.4.2. The issue is traceable to the TransScale.transform.inverted() routine expecting a 2 dimensional array rather than 1d array. If applying this to this older version this should be investigated/tested.

Testing code used (use mouse scroll wheel afterwards):

import matplotlib.pyplot as plt
import matplotlib.backend_tools as bt
import numpy as np

fig1 = plt.figure(0)
zm = bt.ZoomPanBase(fig1,"foo")


img = np.random.random((100,100))
ax = fig1.add_subplot(111)

x = np.linspace(10,1000,1000)
y = 1/x**4

#uncomment the one you want to test, and comment the other
#ax.imshow(img)
ax.loglog(x,y)

def foofunc():
    pass

zm._press = foofunc
zm._release = foofunc

zm.enable("foo")

[tacaswell]

attn @fariza @OceanWolf Can you two take care of reviewing and merging this?

[jrmlhermitte]

@tacaswell , can we hold off for a bit? I just realized that my zooming does not conform to yours.
(Zoom center is center of axes, not the cursor)

Will submit the correction and test when I have time. Thanks

[jrmlhermitte]

@tacaswell
There are three possible zooming conventions:

1. Zoom center is always center of figure (my current version)
2. Zoom center is cursor. Once zoomed, the point under cursor is moved to center of image (current version in this library)
3. Zoom center is cursor. Upon zooming, the point under the cursor does not move.

Which one to use? I prefer number 3 (similar to what you posted here). Have code ready, will submit change once convention is agreed upon.

[WeatherGod]

I have always disliked how zooming happened in matplotlib. I vote for
option 3.

On Thu, Aug 20, 2015 at 10:50 AM, Julien Lhermitte <notifications@github.com

wrote:

@tacaswell https://github.com/tacaswell
There are three possible zooming conventions:

1. Zoom center is always center of figure (my current version)
2. Zoom center is cursor. Once zoomed, the point under cursor is moved to
   center of image (current version in this library)
3. Zoom center is cursor. Upon zooming, the point under the cursor does
   not move.

Which one to use? I prefer number 3. Have code ready, will submit change
once convention is agreed upon.

—
Reply to this email directly or view it on GitHub
#4970 (comment)
.

[jrmlhermitte]

Two votes is good for me. Submitted. Tested on this branch with same code mentioned above.

(math : new axis position: (xa -/+.5_scl + (.5-xa)_scl, then use matplotlib functions to convert to data, which take care of non-linear plots.)

[fariza]

I'll check this next week. I'm on vacation :)
On 20 Aug 2015 12:08 pm, "Julien Lhermitte" notifications@github.com
wrote:

Two votes is good for me. Submitted. Tested on this branch with same code
mentioned above.

(math : new axis position: (xa -/+.5_scl + (.5-xa)_scl, then use
matplotlib functions to convert to data, which take care of non-linear
plots.)

—
Reply to this email directly or view it on GitHub
#4970 (comment)
.

[jrmlhermitte]

enjoy! :-)

[fariza]

@ordirules
I modified your example a little bit to use toolmanager, ZoomPanBase was never supposed to be a tool on it's own (if needed we can change that)

import matplotlib
matplotlib.use('GTK3AGG')
matplotlib.rcParams['toolbar'] = 'toolmanager'
import matplotlib.pyplot as plt
import numpy as np

fig1 = plt.figure(0)
img = np.random.random((100,100))
ax = fig1.add_subplot(111)
x = np.linspace(10,1000,1000)
y = 1/x**4

#uncomment the one you want to test, and comment the other
#ax.imshow(img)
ax.loglog(x,y)

plt.show()

As far as I can test, it works fine!

👍

[OceanWolf]

Ahh, just seen this, a bit busy right now, I will take a look, out of the options, definitely not option 2. Personally I prefer option 3, but I think option 1 also has merits, I think we can revisit it later though (either RcParam or my personal favourite a toggle tool to set zoom mode.

@fariza Type Agg with Camel case.

[jrmlhermitte]

Thanks @fariza for the test code. The previous code was me desperately trying to find a way to test it with my limited knowledge :-D. Glad I could contribute something, even though exceedingly trivial :-D

@OceanWolf
One could easily add a flag for going between 1 and 3, maybe something (add in a kargs and pop it into self.centered):

if(self.centered is not None):
    xa,ya=0.5,0.5
else:
    tranP2A = ax.transAxes.inverted().transform
    xa, ya = tranP2A((event.x, event.y))

or something of the sort. It might never be used but could be good to add. Is it worth it? I could also do it if it saves you time.

[fariza]

@ordirules ToolZoom is doing something similar with the x and y modifiers for zoom_mode.
Just as an idea, what about implementing modifiers in the base class?

[jrmlhermitte]

That makes a lot of sense, very good point. Didn't notice that the others inherit from ZoomPanBase. ToolZoom looks redundant with transScale (probably preceded it ;-) ) so it would make sense to clean up the duplicate code and follow the current transformations philosophy, from my understanding of the tutorial.

The function could be something like "expandshift(ax,x0,y0,x1,y0)" where x,y are the limits in axes coordinates of the new viewbox. I prefer this over "changeview" or something because it emphasizes the relative nature (and potential dangers of looping through).

I think it would also make sense to go deeper and have it part of _AxesBase. The main motivation for this is that it would make life easier for a developer who would want to write their own custom routine without using the GUI. From what I understand (correct me if I'm wrong), but all these inherit from ToolBase which is inherently a GUI feature.

Anyway, what do you think? To sort of summarize:

1. Does such a method already exist to change Axes in _AxesBase?
2. Do you think it's a good idea to add this feature to axes base?

I'll do some reading tonight which may answer my first question (currently busy), but I figured I would ask. Anyway, this is a very good point and I vote for your idea (either case, if it goes in Axes or ZoomPanBase).

[jrmlhermitte]

Hi,

So I took a look at the _AxesBase class and the only function available seems to be set_xlim etc. I think zooming and panning would be appropriate there, since it is more a feature of the axes and not the GUI. (I could imagine other users using this feature without the GUI)

As a suggestion, I would like to propose adding these features to the _AxesBase class:
zoompanx, zoompany, zoompan
zoompan would work as follows, it would zoom the corresponding axis then pan before setting the limits. (We could also add separate zooming and panning features but I think they're often used together and it might add too many functions)
Afterwards, the update of the zooming and panning GUI tools should be straightforward.

@fariza, @OceanWolf, @tacaswell this is a slightly more fundamental proposed change. What are your inputs? I can make the changes no problem but need to know if it is commensurate with current matplotlib philosophy.

thanks! :-)

[jrmlhermitte]

Another note, the main reason to add this to the base class is that it requires transformation from axes coordinates to data. The idea would be to allow the user to have a set of routines to change limits in axes coordinates, without worrying about the transformations to data coordinates. I have made the assumption that the only time you want to change limits in axes coordinates is when you pan or zoom, so I've named it zoompan instead of set_axes_lim or something.

[OceanWolf]

Hmm, before I said I agreed with you, but now not so sure... basically as I see it zoom and pan basically do the same thing, they change the axes limits and they work in a user interactive way... so I think we need to just need a conversion function... perhaps something similar to Basemap.__call__ to convert data coordinates into screen coordinates, but the other way around??

Then again, with different axes types (e.g. 3d) perhaps it becomes easier to do it in the axes itself as it allows the axes to specify its own implementation, rather than making the tool overly complex, catering for all of the different axes types, @WeatherGod you know more about this...

[jrmlhermitte]

Sounds good. I definitely agree with your disagreement with the zoompan idea. :-)
Just to point out a small issue, it seems that drag_pan in _AxesBase does zooming, and it computes everything in data coordinates as well so it might be something to re-evaluate in the future (or maybe not, because it just works well).

Anyway, I want to bring the focus back (sorry for straying away). This was meant to be a patch to fix log-log plots where the zooming currently breaks down and it is currently fixed. How about we make a decision on how to patch it which I believe is one of two options:

1. Leave as is. Fixed point in zoom is cursor point as well. If we really decide to add a zoom center maybe it should be opened as a feature pull request, not patch pull request?
2. Add a base method in ZoomPanBase that ToolZoom which inherits it can also call (the _release method is a little messy, doing the same thing done in scroll_zoom). The overhead makes me more inclined to have it a separate pull request.

I would say to just leave as is for now and open a new pull request that suggests more fundamental changes. If you agree I think we should just commit. What do you think?

thanks again for the feedback!

[jrmlhermitte]

Didn't work for polar coordinate transforms, so changed the way the transformations are done. Also updated ToolZoom to use the transforms available rather than compute them again. There is still an issue w/ the polar coordinate transformation. (I did not realize that commiting to my github would show up here)

Here are the assumptions I make:

• the transAxes transformation is a transformation from the canvas to the normalized bounding box of the coordinates, and not really the axes (true in projections/polar.py as well, see _set_lim_and_transforms)
• there will always be a transformation transAxes and transData, but there is not always a transformation from the normalized bounding box to the data (transLimits, does not exist on polar coordinates)

As of now, the zooming in polar coordinates isn't perfect with the scroll wheel. Sometimes zooming out with the scroll wheel zooms in. Going through all this code is a learning exercise for me. I apologize for all the writing for such a small patch.

Testing code:

import matplotlib
matplotlib.use('GTK3Agg')
matplotlib.rcParams['toolbar'] = 'toolmanager'
import matplotlib.pyplot as plt
from mpl_toolkits.mplot3d import Axes3D
import numpy as np

plt.ion()

fig1 = plt.figure(0)
img = np.random.random((100,100))
ax = fig1.add_subplot(111)

fig2 = plt.figure(1)
ax2 = fig2.add_subplot(111)

fig3 = plt.figure(2)
ax3 = fig3.add_subplot(111,polar="True")

#fig4 = plt.figure(3)
#ax4 = fig4.add_subplot(111,projection="3d")

x = np.linspace(10,1000,1000)
y = 1/x**4

N = 1000
x = np.linspace(-N/2,N/2,N)
y = np.linspace(-N/2,N/2,N)
X,Y = np.meshgrid(x,y)
w = np.where(np.sqrt(X**2 + Y**2) < 10**2)
img = np.zeros((N,N))
img[w] = 1

#uncomment the one you want to test, and comment the other
ax.imshow(img)

ax2.loglog(x,y)

ax3.plot(x,y)

[OceanWolf]

@QuLogic did some work on polar zooming in #4699. I don't have time to compare now, but perhaps you should take a look and compare notes.

[QuLogic]

Maybe you are referring to #4795 actually (which having just been merged is probably causing conflicts here)? Probably ZoomPanBase should use the same API added there. I never tried to update it because I could never get it to work. But of course now I realize I should have used the new toolbar to trigger it...

[jrmlhermitte]

Thanks @QuLogic, I think changing the view from a bounding box is
exactly what is needed!!!! :-) Thanks @OceanWolf for the reference.

I've tried my best to reconcile my code with @QuLogic, and in the
meantime made some small modifications. It works for an image, loglog plot and a simple polar coordinate plot. I think the basic idea is that zooming and panning should be done in the bounding box coordinates, and not the data coordinates. Anything else (saving limits, getting limits, like @QuLogic 's getview and setview can only be done in data coordinates).

Here are the major two changes performed:

• Scroll wheel zooming now works in log-log plots, by simply using @QuLogic's method _set_view_from_bbox.
• I removed the check for the log scaling in the _set_view_from_bbox method and replaced it with the axes transformations in transData. The rectangle selection zooming would be affected by any bugs in this change. I assume that a zoom out with this mode just zooms out by a scale which is the ratio of the widths of the rectangle and the view box, and centers the center of the view box. Honestly, I didn't have time to try to check out to see what the mathematical manipulation in the old code exactly meant but it seems about right?

That's about it.

Here is a quick description of the math for the zooming coordinate calculations which are later
input into @QuLogic's method below. I have not put this in the comments (should I?)

• xcen,xcen (bounding box cen)
• xzc, yzc, center of the selection box (zoom center)
• xd, yd : the data under cursor
  When we select a bounding box, we will basically
  expand/contract by the amount and re-center:
  xdnew = scl_(xd-xzc) + xcen
  but we don't want xdnew to move so set xdnew = xd
  and solve to get:
  xzc = xd_(scl-1) + xcen
  same for ycen

Finally, the test code I have used has not changed from the previous code.
@QuLogic , could you test it rigorously for polar coordinates?

@fariza, so what do you think?

[jrmlhermitte]

I should also point out on line 3339 (x0, y0, x1, y1 = original_view) from axes/_base.py I had to flip y0 and x1. @QuLogic did zoom work for you with this reversal?

[QuLogic]

I should also point out on line 3339 (x0, y0, x1, y1 = original_view) from axes/_base.py I had to flip y0 and x1.

Oh, looking at it, you're probably right; these have been swapped.

[QuLogic]

The point of _get_view/_set_view is to hide the meaning of the "view" from outside of the Axes, because it is not necessarily Cartesian. The following calculation based on it should probably be placed in the Axes.

[jenshnielsen]

Add a warning? or perhaps raise an error

[jrmlhermitte]

yes, good idea. I'll just print a warning and return. Since it's harmless, I think there's no need to raise any exceptions. However, in that case, the user will see quite a few print statements if they repeat the call with wrong (or even just void) arguments. I'm still new to python (been 4 months, coming from yorick, matlab like open source lang and C), so I'm not completely familiar with the pythonic way of error and warning reporting. Open to recommendations.

[QuLogic]

I think we should just remove these.

[jrmlhermitte]

I just fixed for it to conform with PEP8 but the code is out of scope of this PR.
Maybe make a new PR (and I should probably remove the whitespace PEP8 fix)? @fariza ?

[QuLogic]

These sentences could be combined into one.

[jrmlhermitte]

re-worded into something more terse.

[QuLogic]

There's a space at the end here causing PEP8 failures.

[jrmlhermitte]

Thanks @QuLogic. Travis still failing, I'll need to check tonight.
Also as a test noticed a div by zero error and overflow so will catch the exception (and any other region where this could occur) just to be cleaner. has to do with log log plotting and zooming out too much

[jenshnielsen]

I think the last Travis error was just a random fluke. I have restarted

[jrmlhermitte]

Great thanks! I never got to catching and avoiding (by not trying to zoom) the div by zero and overflow exceptions. However, these were not checked for in the original code and it could make sense to have this be a separate PR. Do you think it's worth it?

[fariza]

Is it easy to reproduce?

[jrmlhermitte]

one of them is easy by just zooming in/out way too much. The other I'm not so sure. But I assume by carefully examining it we could probably find a way to test it.

[jrmlhermitte]

oh and thanks @jenshnielsen for the fix and notice!

[fariza]

@ordirules in that case, a new PR is perfect

[OceanWolf]

Can you ping me on that, I feel curious to see how you do that.

[jrmlhermitte]

yeah will do :-)