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

Move widget.{get,set}_active to AxisWidget. #3376

Merged
merged 5 commits into from Sep 11, 2014
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/users/whats_new.rst
Expand Up @@ -382,6 +382,13 @@ instead of ``:context:`` any time you want to reset the context.
Widgets
-------

Active state of Selectors
`````````````````````````

All selectors now implement ``set_active`` and ``get_active`` methods (also
called when accessing the ``active`` property) to properly update and query
whether they are active.

Span Selector
`````````````

Expand Down
64 changes: 30 additions & 34 deletions lib/matplotlib/widgets.py
Expand Up @@ -95,7 +95,7 @@ def __init__(self, ax):
self.ax = ax
self.canvas = ax.figure.canvas
self.cids = []
self.active = True
self._active = True

def connect_event(self, event, callback):
"""Connect callback with an event.
Expand All @@ -111,6 +111,18 @@ def disconnect_events(self):
for c in self.cids:
self.canvas.mpl_disconnect(c)

def set_active(self, active):
"""Set whether the widget is active.
"""
self._active = active

def get_active(self):
"""Get whether the widget is active.
"""
return self._active

active = property(set_active, get_active, doc="Is the widget active?")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdboom @efiring Do we have a style preference on using this method or the decorators?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 2014/08/17, 10:41 AM, Thomas A Caswell wrote:

In lib/matplotlib/widgets.py:

@@ -111,6 +111,18 @@ def disconnect_events(self):
for c in self.cids:
self.canvas.mpl_disconnect(c)

  • def set_active(self, active):
  •    """Set whether the widget is active.
    
  •    """
    
  •    self._active = active
    
  • def get_active(self):
  •    """Get whether the widget is active.
    
  •    """
    
  •    return self._active
    
  • active = property(set_active, get_active, doc="Is the widget active?")

@mdboom https://github.com/mdboom @efiring
https://github.com/efiring Do we have a style preference on using this
method or the decorators?

I don't think this has been discussed. The majority of existing
properties use the function form.

Eric

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer the decorator form, but usually that implies that the
setter and/or getter is doing something non-trivial. The functional form is
nice for trivial setting/getting and can even be used in a templating-ish
fashion.

On Mon, Aug 18, 2014 at 4:48 AM, Eric Firing notifications@github.com
wrote:

In lib/matplotlib/widgets.py:

@@ -111,6 +111,18 @@ def disconnect_events(self):
for c in self.cids:
self.canvas.mpl_disconnect(c)

  • def set_active(self, active):
  •    """Set whether the widget is active.
    
  •    """
    
  •    self._active = active
    
  • def get_active(self):
  •    """Get whether the widget is active.
    
  •    """
    
  •    return self._active
    
  • active = property(set_active, get_active, doc="Is the widget active?")

On 2014/08/17, 10:41 AM, Thomas A Caswell wrote: In
lib/matplotlib/widgets.py: > @@ -111,6 +111,18 @@ def
disconnect_events(self): > for c in self.cids: >
self.canvas.mpl_disconnect(c) > > + def set_active(self, active): > +
"""Set whether the widget is active. > + """ > + self._active = active > +


Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3376/files#r16342384.


def ignore(self, event):
"""Return True if event should be ignored.

Expand Down Expand Up @@ -1081,7 +1093,21 @@ def _update(self):
self.canvas.draw_idle()


class SpanSelector(AxesWidget):
class _SelectorWidget(AxesWidget):
def set_active(self, active):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should over-ride the property as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tacaswell, is this the only thing holding this PR up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looking at this again, it does update, the property method is tricksy.

super(_SelectorWidget, self).set_active(active)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a good of an idea is it seems, because we aren't using super every where can you hard-code the over loading?

The reason I am worried is that if this ends up in an MRO where not everything is cooperating, you can get strange bugs. If we are going to go down this route, everything in the AxesWidget class strucutre also needs to use super.

if active:
self.update_background(None)

def update_background(self, event):
"""force an update of the background"""
# If you add a call to `ignore` here, you'll want to check edge case:
# `release` can call a draw event even when `ignore` is True.
if self.useblit:
self.background = self.canvas.copy_from_bbox(self.ax.bbox)


class SpanSelector(_SelectorWidget):
"""
Select a min/max range of the x or y axes for a matplotlib Axes.

Expand Down Expand Up @@ -1189,13 +1215,6 @@ def new_axes(self, ax):
if not self.useblit:
self.ax.add_patch(self.rect)

def update_background(self, event):
"""force an update of the background"""
# If you add a call to `ignore` here, you'll want to check edge case:
# `release` can call a draw event even when `ignore` is True.
if self.useblit:
self.background = self.canvas.copy_from_bbox(self.ax.bbox)

def ignore(self, event):
"""return *True* if *event* should be ignored"""
widget_off = not self.visible or not self.active
Expand Down Expand Up @@ -1302,7 +1321,7 @@ def onmove(self, event):
return False


class RectangleSelector(AxesWidget):
class RectangleSelector(_SelectorWidget):
"""
Select a rectangular region of an axes.

Expand Down Expand Up @@ -1393,7 +1412,6 @@ def __init__(self, ax, onselect, drawtype='box',
self.connect_event('button_release_event', self.release)
self.connect_event('draw_event', self.update_background)

self.active = True # for activation / deactivation
self.to_draw = None
self.background = None

Expand Down Expand Up @@ -1437,11 +1455,6 @@ def __init__(self, ax, onselect, drawtype='box',
# will save the data (pos. at mouserelease)
self.eventrelease = None

def update_background(self, event):
"""force an update of the background"""
if self.useblit:
self.background = self.canvas.copy_from_bbox(self.ax.bbox)

def ignore(self, event):
"""return *True* if *event* should be ignored"""
if not self.active:
Expand Down Expand Up @@ -1575,19 +1588,8 @@ def onmove(self, event):
self.update()
return False

def set_active(self, active):
"""
Use this to activate / deactivate the RectangleSelector
from your program with an boolean parameter *active*.
"""
self.active = active

def get_active(self):
""" Get status of active mode (boolean variable)"""
return self.active


class LassoSelector(AxesWidget):
class LassoSelector(_SelectorWidget):
"""Selection curve of an arbitrary shape.

For the selector to remain responsive you much keep a reference to
Expand Down Expand Up @@ -1679,12 +1681,6 @@ def onmove(self, event):
else:
self.canvas.draw_idle()

def update_background(self, event):
if self.ignore(event):
return
if self.useblit:
self.background = self.canvas.copy_from_bbox(self.ax.bbox)


class Lasso(AxesWidget):
"""Selection curve of an arbitrary shape.
Expand Down