Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add inaxes method to FigureCanvas to check whether point is in an axes.#9845
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
As noted in#9821 I'd rather 1. make Artist.contains be able to take xy input + 2. expose cbook._topmost_artist as public API, which should together provide a much more general solution. In order to disambiguate whether xy is in figure or in axes or in data coordinates, we could e.g. fiddle with kwargs to support |
Problem is how should a user know if there exist a topmost artist method? Heck everything can be done with artist.contains_points. And do some transformasjon between coordinates. Thus the only thing that is really needed is just to expose cbook._topmost_artist. |
In any cases a specific method is not mutually exclusive with general case. I believe for new user which is not matplotlib guru to find this method useful. |
"How does the user know about topmost_artist" and "how does the user know about inaxes" are essentially the same problem -- discoverability of rarely used functions. I'd rather document a more widely useable function (topmost_artist), possibly with some examples in the docstring that show how the function can be used to solve the inaxes problem, rather than add yet another very specialized tool. |
No it is two different problems. If you have an issue related to figure you go into the Figure class and look for its method. You do not go and look in artist (assume you are putting it there). The inaxes method is specialized method because it does what it should do. If you really want to find it usage then just use it in LocationEvent. |
lib/matplotlib/backend_bases.py Outdated
| @@ -1531,15 +1531,9 @@ def __init__(self, name, canvas, x, y, guiEvent=None): | |||
| self._update_enter_leave() | |||
| return | |||
| # Find all axes containing the mouse | |||
| if self.canvas.mouse_grabber is None: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You probably don't want to lose the mouse-grabbing effect.
My point is that this has in fact nothing to do with axes and figures: what you want to know is rather what artist (if any), among a list of artists, is the topmost at a point in the canvas... (and I am all for documentation that makes {contains + topmost_artist} more discoverable). |
Just because you do not find its usage I just used it in LocationEvent. |
lib/matplotlib/figure.py Outdated
| axes_list = [a for a in self.get_axes() if a.patch.contains_point(xy)] | ||
| if axes_list: | ||
| axes = max(reversed(axes_list), key=lambda x: x.zorder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For consistency we probably should use_topmost_artist here (so if we ever need to change that logic we can do it exactly once).
@anntzer Instead of having a zoo of kwargs, we probably should follow the pattern of taking a @lkjell This method may make more sense on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@tacaswell your point of adding a transform kwarg to contains (for example) is well taken.
I really do not believe this method should be added, so I am going to reject the changes. Other reviewers should feel free to dismiss this review if they do not consider my arguments convincing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this is a useful thing to have factored out and exposed and is a reasonable implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This looks good to me. Needs a rebase
Return the top-most axes if found, else None.
@tacaswell and@jklymak since there are 2 approvals it can be merged now? |
Merging over@anntzer disapproval. This tool doesn't have a lot of complicated machinery associated with it, and seems like it is a nice shortcut. I agree that we don't want to have a bunch of duplicated methods but in the absence of the work noted above exposing the API, this seems like a harmless stopgap. |
This broke the build, so suggesting we revert for now.@lkjell if you care to rebase and figure out what broke, that would be fine... |
Uh oh!
There was an error while loading.Please reload this page.
Add inaxes method to FigureCanvas to check whether point is in an axes.
Return the top-most axes if found, else None.
PR Summary
An alternative way to get an axes given (x,y) coordinate is to create a LocationEvent and use the inaxes attribute. However, instantiate LocationEvent may trigger axes_enter_event and axes_leave_event, which is not one often would like to do. With this method a new way to find the axes without triggering axes_enter_event and axes_leave_event.
From issue#9821.
PR Checklist