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

trying out mpl-pan-zoom #3

Open
robert-kozar-nnl opened this issue Jul 21, 2022 · 5 comments
Open

trying out mpl-pan-zoom #3

robert-kozar-nnl opened this issue Jul 21, 2022 · 5 comments

Comments

@robert-kozar-nnl
Copy link

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
Copy link
Collaborator

ianhi commented Jul 21, 2022

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
Copy link
Author

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
Copy link
Collaborator

ianhi commented Jul 25, 2022

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
Copy link
Collaborator

ianhi commented Jul 25, 2022

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants