mpl-extensions / mpl-pan-zoom

Zooming and Panning for Matplotlib plots
https://mpl-pan-zoom.rtfd.io
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

trying out mpl-pan-zoom #3

Open robert-kozar-nnl opened 2 years ago

robert-kozar-nnl commented 2 years ago

Ian, It was nice meeting and talking with you during the sprints at SciPy. I've been trying to implement mpl-pan-zoom in the GUI that I was showing you and Elliot. I got the zoom factory to work, but I'm not sure how to activate the pan feature. This is a stripped down version of what I thought should work. Am I missing something? -Rob

  import matplotlib.pyplot as plt
  from mpl_pan_zoom import zoom_factory, PanManager

  fig, ax = plt.subplots()
  plt.plot([0, 1, 2], [2, 1, 0], 'o')
  PanManager(fig, 1)
  zoom_factory(ax)
  plt.show()
ianhi commented 2 years ago

Hi Robert, was great to meet you as well!

Am I missing something?

Yup one small thing, you need to keep a reference to the PanManager so it does not get garbage collected and lose all it's callbacks. So change the PanManager line to read:

pm = PanManager(fig, 1)

should fix things.

This is an easy thing to miss, especially when I didn't link to the docs anywhere on this repo... that's my bad. Link is here: https://mpl-pan-zoom.readthedocs.io/en/latest/#scrolling-and-panning

Out of curiosity do you think the note here: https://mpl-pan-zoom.readthedocs.io/en/latest/#scrolling-and-panning would have been noticeable enough for you to pick up on this or should we change it to make it stand out even more?

robert-kozar-nnl commented 2 years ago

That makes perfect sense and I should have thought of that. Works as expected now.

As for the documentation, I think that covers it. Even without the note, just the line pan_manager = PanManager(fig, MouseButton.MIDDLE) would have cued me in to my oversight.

Since I'm embedding it in a pyqt5 widget as we discussed (That is working great by the way. Coupled with blitting it completely fixed my responsiveness issue!), it didn't work unless I made pan_manager an attribute of the figure window class, but I think that should be common knowledge for people using it in that way.

One thing I noticed while implementing the method is that the PanManager class says the default value for button is right click, but no default is given in the init (e.g. button=3), so if a value for button isn't supplied, the following error occurs: TypeError: init() missing 1 required positional argument: 'button'

ianhi commented 2 years ago

One thing I noticed while implementing the method is that the PanManager class says the default value for button is right click, but no default is given in the init (e.g. button=3), so if a value for button isn't supplied, the following error occurs:

Ah good catch thank you! That's an oversight from when I copied the code over. I moved to making it a required argument because i think use cases vary enough that I shouldn't be involved in prescribing a default.

Anyone who sees this is welcome to open a pull request to update the doc string, otherwise I'll hopefully be able to do it once I'm done with the conference I'm at

ianhi commented 2 years ago

Since I'm embedding it in a pyqt5 widget as we discussed (That is working great by the way. Coupled with blitting it completely fixed my responsiveness issue!)

Awesome! Really glad to hear it :). When you've got something public I'd love to see it, looked really interesting

robert-kozar-nnl commented 2 years ago

Sounds good. I think it makes sense to force the user to make a conscious decision about which button to assign the pan to since there is a high likelihood that it could conflict with other functionalities.

I edited the docstring to remove the mention of a default, but my work account wouldn't let me push to remote, and I don't want to reconfigure my GitHub settings to my personal account right now.