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

Lasso selector #730

Merged
merged 13 commits into from Mar 17, 2012

Conversation

Projects
None yet
3 participants
@tonysyu
Copy link
Contributor

tonysyu commented Mar 1, 2012

Add LassoSelector widget that behaves more like RectangleSelector and SpanSelector than the current Lasso widget.

Note: I put the demo in the "widgets" directory even though the Lasso demo is in "event_handling".

Also, note that this PR builds off of PR #724: Only the last commit in this PR adds the new widget.

tonysyu added some commits Feb 27, 2012

Remove call to `new_axes` in SpanSelector.
* This change is in preparation for generalizing the base Widget.
* It's not clear to me why `new_axes` is necessary, but I left it in (for now).
Add AxesWidget class and let it initialize `ax` and `canvas` attributes.
* Renamed `Lasso.axes` to `Lasso.ax` for consistency.
* Remove `Lasso.figure`, which was unused.
Add default ignore method and check it in callbacks.
* Attribute name "active" can be confused with parameter to `RadioButtons`, but `RadioButtons` doesn't save that parameter as an attribute, so there's no name clash. BUT this is still really confusing to users. Consider renaming `AxesWidget.active`.
*
* * This could have been implemented as a decorator, but some callbacks wouldn't be compatible. For example, `SpanSelector.release` should not be ignored if widget was deactivated during mouse press.
*
* * `SpanSelector` and `RectangleSelector` already had `ignore` methods, but they do not call it in `update_background`. I don't change the current behavior, although it seems desirable.
Fix: save correct callback ids.
Also, remove unused import.
Add LassoSelector widget with demo.
Note: I put the demo in the "widgets" directory even though the `Lasso` demo is in "event_handling".
@@ -64,9 +63,41 @@ class Widget(object):
eventson = True


class AxesWidget(Widget):
"""
Widget that is connected to a single :class:`Axes`.

This comment has been minimized.

@pelson

pelson Mar 1, 2012

Member

Does this render correctly? It is certainly safer to do :class:~matplotlib.axes.Axes. (the tilde shortens the link to Axes)

This comment has been minimized.

@tonysyu

tonysyu Mar 2, 2012

Contributor

I didn't know about this. Thanks for the tip.

import numpy as np

from matplotlib.widgets import LassoSelector
from matplotlib.nxutils import points_inside_poly

This comment has been minimized.

@pelson

pelson Mar 2, 2012

Member

That can't work since nxutils was removed in master... see #732 for example of how to get around that one.

This comment has been minimized.

@tonysyu

tonysyu Mar 4, 2012

Contributor

Hmm, I'm on the latest master, and it works fine on my system. Am I missing something?

This comment has been minimized.

@tonysyu

tonysyu Mar 12, 2012

Contributor

Actually,... I didn't clean out my build, so I had a leftover nxutils.so. I just converted to use Path.contains_point. Thanks for the tip!

@@ -133,6 +171,8 @@ def __init__(self, ax, label, image=None,
self._lastcolor = color

def _click(self, event):
if self.ignore(event):

This comment has been minimized.

@pelson

pelson Mar 2, 2012

Member

These would make nice decorators... :-)

This comment has been minimized.

@tonysyu

tonysyu Mar 4, 2012

Contributor

Actually, my first implementation used decorators. At the time, I called the decorator callback and applied it to every callback event. But then I noticed that you want to be more careful with a handful of cases (for example, you want to be able to release SpanSelector even if you deactivate the widget in-between press and release events---not really sure how likely this is, but the code checked for it). When I realized that the callback decorator wouldn't be called on all the callbacks, I thought it might get confusing.

I guess renaming the decorator to check_ignore (or similar) might make the functionality a bit clearer. My other hesitation was that it may be weird for a decorator to call a class method---seems somewhat magical. I'd be fine with it though.

@efiring

This comment has been minimized.

Copy link
Member

efiring commented Mar 12, 2012

Please add docstrings for Lasso and LassoSelector.

@tonysyu

This comment has been minimized.

Copy link
Contributor

tonysyu commented Mar 12, 2012

@efiring doctstrings, done.

efiring added a commit that referenced this pull request Mar 17, 2012

@efiring efiring merged commit adf6680 into matplotlib:master Mar 17, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment