-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add AxesWidget base class to handle event activation and clean up. #724
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
Conversation
* 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).
* Renamed `Lasso.axes` to `Lasso.ax` for consistency. * Remove `Lasso.figure`, which was unused.
* 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.
|
||
def connect_event(self, event, callback): | ||
self.canvas.mpl_connect(event, callback) | ||
self.cids.append(callback) |
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 is wrong. The cids are not the callback functions themselves, they are the integers returned by the call to mpl_connect().
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.
Good catch! I had this correct at some point, but somehow, I broke it along the way.
Overall, I like the idea, and I do see its value in bringing uniformity to the widgets. This does open the doors to subsequent updates. |
Also, remove unused import.
Class links were incorrect.
connect_event
method to connect event and store a reference. Additionaldisconnect_events
method allows user to clean up widget. Some widgets did their own cleanup, but this makes behavior more consistent.ignore
method and call it in event callbacks. Some widgets had their ownignore
methods, but this makes behavior more consistent.Todo:
SpanSelector.new_axes
should be removed, but may be some user's code depends on it. Right now I duplicate most of thenew_axes
code in__init__
: Ifnew_axis
isn't removed, then duplication should be removed.update_background
inSpanSelector
andRectangleSelector
don't ignore events when deactivated.