Improved selection widget by blink1073 · Pull Request #3502 · matplotlib/matplotlib

blink1073 · 2014-09-12T02:39:09Z

This is in preparation for enhancing the selection widgets as per #3486.

This expands the new _Selector class from #3376 and normalizes the API for the three current selection widgets: RectangleSelector, LassoSelector, and SpanSelector.

The only known API change is that the ignore criteria from the RectangleSelector are now used for all three, since they were the most complete and robust.

This also adds tests for all three widgets.

blink1073 · 2014-09-12T02:40:36Z

@tacaswell, @fariza, here we go...

fariza · 2014-09-12T03:37:55Z

@blink1073
For the moment I would keep the draw_rubberband out of this and get everything in shape.
It can be done later with another PR.

Just for future reference
The draw_rubberband can not be "hard coded" here, it has to be implemented in each backend (it's already like that). By the way the method that you used is from the qt backend, it is different for each backend.
The idea to use the draw_rubberband method is to be faster, better, etc.. (#3475 (comment)) and use it (if available) instead of the matplotlib.patches.Rectangle that you are using in RectangleSelector

blink1073 · 2014-09-12T11:08:24Z

I removed draw_rubberband. I see now that I will have to use self.canvas.toolbar.draw_rubberband when I integrate that behavior.

tacaswell · 2014-10-18T17:09:20Z

@blink1073 This needs (at minimum) a re-base.

blink1073 · 2014-10-19T03:38:48Z

Rebase complete.

tacaswell · 2014-10-19T05:02:34Z

lib/matplotlib/tests/test_widgets.py

GUI tests? madness!

Ha, I had already written these tests for scikit-image, but yes, it was a pain...

You need to white-list this file to be run in

matplotlib/lib/matplotlib/__init__.py

Line 1397 in c600130

default_test_modules = [

jenshnielsen · 2014-11-09T11:31:35Z

Some tests are failing on Travis

blink1073 · 2014-11-09T22:35:42Z

I'm at a bit of a loss here. I tested on both Python 2.7 and 3.4 with Ubuntu 14.04 64bit. The build is not only failing two of my tests but some outside of the reach of this PR as well: mpl_toolkits.tests.test_mplot3d.test_bar3d.test. I ran the tests on my machine using nosetests test_widget.py. I'm having a really hard time following the magic incantation you're using in your .travis.yml file.

tacaswell · 2014-11-09T23:24:49Z

lib/matplotlib/tests/test_widgets.py

Add a @cleanup (from matplotlib.testing.decorators) to this. I suspect that the other failing tests are due the artists form this test leaking out.

tacaswell · 2014-11-10T11:59:25Z

I can reproduce the failures locally running the tests like they are run on travis:

(conda27)tcaswell@eowyn:matplotlib$ python tests.py matplotlib.tests.test_widgets
FEF
======================================================================
ERROR: matplotlib.tests.test_widgets.test_span_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 135, in test_span_selector
    check_span('horizontal', minspan=10, useblit=True)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 128, in check_span
    assert ax._got_onselect
AttributeError: 'AxesSubplot' object has no attribute '_got_onselect'

======================================================================
FAIL: matplotlib.tests.test_widgets.test_rectangle_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 94, in test_rectangle_selector
    check_rectangle(useblit=True, button=1)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 83, in check_rectangle
    tool.onmove(event)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1618, in onmove
    self.update()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1195, in update
    self.ax.draw_artist(artist)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/axes/_base.py", line 2103, in draw_artist
    assert self._cachedRenderer is not None
AssertionError

======================================================================
FAIL: matplotlib.tests.test_widgets.test_lasso_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 164, in test_lasso_selector
    check_lasso_selector()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 155, in check_lasso_selector
    tool.onmove(event)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1714, in onmove
    self.update()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1195, in update
    self.ax.draw_artist(artist)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/axes/_base.py", line 2103, in draw_artist
    assert self._cachedRenderer is not None
AssertionError

----------------------------------------------------------------------
Ran 3 tests in 0.484s

FAILED (errors=1, failures=2)

I get (slightly different) failures if I run just the widget tests via nosetest:

(conda27)tcaswell@eowyn:tests$ nosetests test_widgets.py
FEF
======================================================================
ERROR: matplotlib.tests.test_widgets.test_span_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 135, in test_span_selector
    check_span('horizontal', minspan=10, useblit=True)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 123, in check_span
    tool.onmove(event)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1427, in onmove
    self.update()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1196, in update
    self.canvas.blit(self.ax.bbox)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/backends/backend_qt5agg.py", line 158, in blit
    self.repaint(l, self.renderer.height-t, w, h)
AttributeError: 'FigureCanvasQTAgg' object has no attribute 'renderer'

======================================================================
FAIL: matplotlib.tests.test_widgets.test_rectangle_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 94, in test_rectangle_selector
    check_rectangle(useblit=True, button=1)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 83, in check_rectangle
    tool.onmove(event)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1618, in onmove
    self.update()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1195, in update
    self.ax.draw_artist(artist)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/axes/_base.py", line 2103, in draw_artist
    assert self._cachedRenderer is not None
AssertionError

======================================================================
FAIL: matplotlib.tests.test_widgets.test_lasso_selector
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/conda27/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 164, in test_lasso_selector
    check_lasso_selector()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/testing/decorators.py", line 110, in wrapped_function
    func(*args, **kwargs)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/tests/test_widgets.py", line 155, in check_lasso_selector
    tool.onmove(event)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1714, in onmove
    self.update()
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/widgets.py", line 1195, in update
    self.ax.draw_artist(artist)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/axes/_base.py", line 2103, in draw_artist
    assert self._cachedRenderer is not None
AssertionError

----------------------------------------------------------------------
Ran 3 tests in 0.562s

FAILED (errors=1, failures=2)

blink1073 · 2014-11-10T12:02:03Z

Funny, now that I added the @cleanup decorators, I get the errors locally as well, investigating...

tacaswell · 2014-11-10T12:08:56Z

lib/matplotlib/widgets.py

This smells like something that is going to introduce race-condition/transient bugs if more than one callback is using the same event.

Should events have a lock attribute perhaps? Then you could use with event.lock():.

I found the right solution: use a utility method to calculate the bounds as needed, and use a copy of the events for the onselect callback.

blink1073 · 2014-11-11T15:35:31Z

Passing all tests now.

tacaswell · 2014-11-11T16:25:16Z

Catching AssertionErrors makes me a little nervous as the asserts can be optimized out (in principle...does anyone actually use pyo?) which will presumably cause AttributeErrors or similar later. draw_artist should be changed to raise a different type of Exception in case of the cached renderer being None, probably a custom exception....

blink1073 · 2014-11-11T16:58:02Z

Sure, but I have no idea what the error means or what to do about it...

tacaswell · 2014-11-11T18:24:26Z

@blink1073 See PR against your branch.

blink1073 · 2014-11-11T21:14:57Z

@tacaswell, I am offended and pleased by your GIF.

tacaswell · 2014-11-11T21:26:26Z

Why offended?

…ted.

- strip white space - rearrange 'not's

Widgets assume that the canvas has been drawn atleast once before they are called (which is reasonable for interactive usage as the user needs to be able to _see_ the gui before they can interact with it). Ensure that the tests draw the canvas before trying to use the widgets. Doing this allows for removing some of the error handling logic from the widgets.

blink1073 · 2014-11-12T11:44:29Z

@tacaswell, assuming I didn't dork up the rebase, this should be good to go.

blink1073 · 2014-11-12T19:16:37Z

@tacaswell, it looks like the build failed due to some unrelated network problems, do you want to restart it?

ENH : selection widget refactor

blink1073 · 2014-11-13T01:08:46Z

What's next, the new RectangleSelector?

tacaswell · 2014-11-13T17:16:29Z

If by new you mean moving over what you have in skimage, then yes!

On Wed Nov 12 2014 at 8:08:47 PM Steven Silvester notifications@github.com
wrote:

What's next, the new RectangleSelector?

—
Reply to this email directly or view it on GitHub
#3502 (comment)
.

tacaswell added this to the v1.5.x milestone Sep 12, 2014

tacaswell added the status: needs review label Sep 12, 2014

tacaswell added status: needs revision and removed status: needs review labels Oct 18, 2014

blink1073 force-pushed the improved-selection-widget branch from 8e94df1 to a68d119 Compare October 19, 2014 03:38

tacaswell added status: needs review and removed status: needs revision labels Oct 19, 2014

tacaswell reviewed Oct 19, 2014
View reviewed changes

jenshnielsen added status: needs revision and removed status: needs review labels Nov 9, 2014

blink1073 force-pushed the improved-selection-widget branch from ae3a08e to bdc8800 Compare November 9, 2014 17:28

tacaswell reviewed Nov 9, 2014
View reviewed changes

tacaswell reviewed Nov 10, 2014
View reviewed changes

blink1073 and others added 18 commits November 12, 2014 05:43

Factor more behavior into the _SelectorWidget class.

61ab6df

Fix indentation

e43c788

Put onselect in __init__, fix syntax error, make sure lines are anima…

ea732ef

…ted.

Add tests for the selection widgets

900a6ea

Remove private event handling methods to allow for subclassing old api

706b5b4

Remove draw_rubberband and minor cleanup

ffd715a

Add to default_test_modules

53b6056

Attempt to fix affine transform error

ddfcf59

Fix affine transform error

76721c1

Add test cleanup decorator

cf66812

Move cleanup to the individual checks

babc377

Fix failing tests

cf58c0a

Fix pep8 error

f3261cf

Improve handling of out-of-bounds releases

c25efbd

Revert changes to setupext.py

ce93c98

Style change and use self.blit instead of blit.

51c70f0

MNT : pep8 + pyflake changes

d711940

- strip white space - rearrange 'not's

blink1073 force-pushed the improved-selection-widget branch from dfcb0b7 to 86f28fa Compare November 12, 2014 11:43

tacaswell added a commit that referenced this pull request Nov 13, 2014

Merge pull request #3502 from blink1073/improved-selection-widget

0efdf2f

ENH : selection widget refactor

tacaswell merged commit 0efdf2f into matplotlib:master Nov 13, 2014

blink1073 deleted the improved-selection-widget branch November 13, 2014 01:08

jenshnielsen mentioned this pull request Nov 13, 2014

Fix Sphinx warning in widgets #3788

Merged

tacaswell mentioned this pull request Nov 26, 2014

spanselector does not respect widgetlock #1368

Closed

blink1073 mentioned this pull request Dec 20, 2014

Rectangle Selector Upgrade #3937

Merged

QuLogic removed the status: needs revision label Nov 24, 2016

Uh oh!

Conversation

blink1073 commented Sep 12, 2014

Uh oh!

blink1073 commented Sep 12, 2014

Uh oh!

fariza commented Sep 12, 2014

Uh oh!

blink1073 commented Sep 12, 2014

Uh oh!

tacaswell commented Oct 18, 2014

Uh oh!

blink1073 commented Oct 19, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenshnielsen commented Nov 9, 2014

Uh oh!

blink1073 commented Nov 9, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Nov 10, 2014

Uh oh!

blink1073 commented Nov 10, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Nov 11, 2014

Uh oh!

tacaswell commented Nov 11, 2014

Uh oh!

blink1073 commented Nov 11, 2014

Uh oh!

tacaswell commented Nov 11, 2014

Uh oh!

blink1073 commented Nov 11, 2014

Uh oh!

tacaswell commented Nov 11, 2014

Uh oh!

blink1073 commented Nov 12, 2014

Uh oh!

blink1073 commented Nov 12, 2014

Uh oh!

blink1073 commented Nov 13, 2014

Uh oh!

tacaswell commented Nov 13, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants