The test failures look like they are coming out of bar3D tests which I suspect are unrelated.

@tacaswell I believe I addressed your concern about return values.

Down to pep8 issues:

======================================================================

FAIL: matplotlib.tests.test_coding_standards.test_pep8_conformance_installed_files

----------------------------------------------------------------------

Traceback (most recent call last):

File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/nose/case.py", line 197, in runTest

self.test(*self.arg)

File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py", line 255, in test_pep8_conformance_installed_files

expected_bad_files=expected_bad_files)

File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py", line 140, in assert_pep8_conformance

assert_equal(result.total_errors, 0, msg)

AssertionError: Found code syntax errors (and warnings):

/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_widgets.py:190:27: E222 multiple spaces after operator

/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_widgets.py:196:27: E222 multiple spaces after operator

----------------------------------------------------------------------

This fell off the bottom of my internal FILO buffer, what is the state of this?

The last few commits were some cleanup as a result of using this branch to make the contrast selector tool. I'd say its done and ready.

Current priority is getting 1.4.3rc1 cut, hopefully I will be able to loop back to the master branch stuff 'soon'.

No problemo.

@blink1073 it's been a long time, but did you test this changes with the new toolmanager introduced in MEP22?
I foresee some problems with the key_press_event that ToolManager also uses to trigger tools

@fariza, I just did a rebase and played with the RectangleSelect widget, it is working as intended.

My concern comes from #1368
At the same time, ToolManager adds a keypresslock to redirect keypress events, and these widgets use the event to mvoe around the selection.
I was thinking in moving the keypresslock to the canvas, and have everybody using it widgets and toolmanager so there are no tools triggered unintentionally

That makes sense to me.

I just tried and I noticed a little change in the behaviour.
With this change the selection is not cleared after release, is this the correct thing to do?

Sorry, I just read the description.
Running the example rectangle_selector.py there are no handles to modify the shape, I think if we are going to modify the widget, we have to modify the example to reflect the changes accordingly.
And, if the selection is going to stay in place after using it, what would be the normal way to remove it?

@fariza, assuming @tacaswell agrees with the new behavior, I can update the example. One would clear the selection by clicking outside of the selection (without dragging).

Wouldn't it be better to add a kwarg to persist after release?
Given that you are capturing key_press_events what about adding the 'esc' key to remove the selection?
If I'm not wrong this widget is not using the ignore method to prevent collisions

I agree with both statements: preserve the previous behavior and allow the escape key to clear the selection.

Ah, I remember now.  It is "control" if it is the only key and "ctrl" in a key combination.  I can just account for that one special case. 

How about a state_keys dictionary for a _Selection widget, where you can override the default states or provide new ones?

Interesting idea. It would help keep the constructor from getting cluttered.

On Tue, Aug 25, 2015 at 8:32 AM, Steven Silvester notifications@github.com
wrote:

How about a state_keys dictionary for a _Selection widget, where you can
override the default states or provide new ones?

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

@blink1073 I like the interactive kwarg.
In the interactive mode, if I press Esc it dissapears but reapears after I click twice anywhere in the axes, is this the intended behavior?

@WeatherGod @tacaswell There are several places around the code, that we hardcoded keys to specific functions.
We have to get a standardized way of doing this. In the case of toolmanager the tools can capture key_press_event and do stuff inside, but so far there is no way of modifying this.
I kind of like the idea of a kwarg with a dictionary. But I would be careful with the naming, this has to be easily transportable to all the widgets and tools, state_keys is not easily identifiable.
I guess we need a keys_compat.py somewhere to deal with different keys in different backends, and force widget and tools creators to use that when they specify default keys.

I settled on state_modifier_keys, how does that sound?
Also, I fixed the double-click zombie shape behavior.

Both test failures looked like the tex related transients.

@fariza I agree re the need for a centralized way to manage key event bindings, but want to punt on that for this PR.

@WeatherGod If you are happy, please merge this.

@blink1073 Can you add a whats_new in a new PR?

@WeatherGod Merged this as no activity for a week, we can iterate in smaller PRs if needed.

@tacaswell, indeed. Sorry for the silence on my end as other things came up. Unfortunately, I don't see much relief in sight over the next few weeks.

@tacaswell, which one would you like next? I was thinking LassoSelector.

LassoSelector sounds good to me.

On Tue, Sep 8, 2015 at 9:33 AM Steven Silvester notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell, which one would you like next?
I was thinking LassoSelector.

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

This builds on #3486 and #3502.

This improves the RectangleSelector and adds an EllipseSelector with minimal code differences.
Improvements:

• Visible handles for manipulating the shape after it has been drawn
• Keyboard modifiers:
• Alt or Space moves the existing shape (as does dragging the center handle)
• Shift makes the shape square (or circular)
• Ctrl makes the initial point the center of the shape (helpful for ellipses)
• Ctrl and shift can be combined

The API is the same (unless using the modifier keys), but we add the interactive keyword arg to enable the new behavior.

How to remove Rectangle Selector from the figure? Is it possible?