Fixes zoom rubberband display on macOS w/ wxagg and multiple subplots #9005
Conversation
…S when using the wxagg backend and multiple subfigures.
| event.inaxes.bbox) | ||
| self.zoomStartX = event.xdata | ||
| self.zoomStartY = event.ydata | ||
| self.zoomAxes = event.inaxes |
There was a problem hiding this comment.
Can you make this a private member (precede it with an underscore)? Also, we should probably clear it when the rubber-banding is finished so we don't hang onto the axes reference.
There was a problem hiding this comment.
Thats easy enough to do. However, making it private makes it inconsistent with the zoomStart variables, which is why I left it as is. I could also change those, but wasn't sure if they were public for a reason. Thoughts?
There was a problem hiding this comment.
No clue if they were public for any particular reason except legacy. We should keep those two as-is, but the zoomAxes should be private under the philosophy that it is easier to make something public later, than to make it private. If it doesn't need to be public, then default to private for anything new.
There was a problem hiding this comment.
In this case I think it makes more senses to be consistent with the existing group of names on the 'when-in-rome' principle.
Lets leave everything as 'public' for now.
|
Still need to release a reference to the axes when rubber-banding is done. |
|
@jbhopkins Thanks for bearing with us on this in your first Matplotlib PR! |
|
Happy to help, @tacaswell . |
|
@jbhopkins Thank you and congratulations on your first Matplotlib PR 🎉 ! Hopefully we will hear from you again! |
PR Summary
This fixes an issue where the zoom box (rubberband) was not displaying correctly when using the wxagg backend on macOS/OS X and a figure had multiple subplots.
It is a fix for the following issue:
#9004
PR Checklist