Re: [matplotlib-devel] faulty design for Lasso widget?

[Benjamin R. <ben...@ou...>]

On Tue, Feb 28, 2012 at 1:00 PM, Tony Yu <ts...@gm...> wrote:

>
>
> On Tue, Feb 28, 2012 at 11:44 AM, Benjamin Root <ben...@ou...> wrote:
>
>> On Mon, Feb 27, 2012 at 9:45 PM, Tony Yu <ts...@gm...> wrote:
>>
>>>
>>>
>>> On Fri, Feb 17, 2012 at 12:17 PM, Benjamin Root <ben...@ou...> wrote:
>>>
>>>>
>>>>
>>>> On Fri, Feb 17, 2012 at 11:06 AM, Ryan May <rm...@gm...> wrote:
>>>>
>>>>> On Fri, Feb 17, 2012 at 10:14 AM, Benjamin Root <ben...@ou...>
>>>>> wrote:
>>>>> > Hello all,
>>>>> >
>>>>> > I tracked down an annoying problem in one of applications to the
>>>>> Lasso
>>>>> > widget I was using.  The widget constructor lets you specify a
>>>>> function to
>>>>> > call when the lasso operation is complete.  So, when I create a
>>>>> Lasso, I set
>>>>> > the canvas's widget lock to the new lasso, and the release function
>>>>> will
>>>>> > unlock it when it is done.  What would occassionally happen is that
>>>>> the
>>>>> > canvas wouldn't get unlocked and I wouldn't be able to use any other
>>>>> widget
>>>>> > tools.
>>>>> >
>>>>> > It turns out that the release function is not called if the number of
>>>>> > vertices collected is not more than 2.  So, accidental clicks that
>>>>> activate
>>>>> > the lasso never get cleaned up.  Because of this design, it would be
>>>>> > impossible to guarantee a proper cleanup.  One could add another
>>>>> > button_release callback to clean up if the canvas is still locked,
>>>>> but there
>>>>> > is no guarantee that that callback is not called before the lasso's
>>>>> > callback, thereby creating a race condition.
>>>>> >
>>>>> > The only solution I see is to guarantee that the release callback
>>>>> will be
>>>>> > called regardless of the length of the vertices array.  Does anybody
>>>>> see a
>>>>> > problem with that?
>>>>>
>>>>> Not having looked at the Lasso code, wouldn't it be possible to use
>>>>> one internal callback for the button_release event, and have this
>>>>> callback call the users' callbacks if points > 2 and always handle the
>>>>> unlocking of the canvas?
>>>>>
>>>>> Ryan
>>>>>
>>>>>
>>>> The problem is that the constructor does not establish the lock.  It is
>>>> the user's responsibility to establish a lock and release the locks for
>>>> these widgets.  Plus, if the user's callback has cleanup code (such as mine
>>>> did), not guaranteeing that the callback is done can leave behind a mess.
>>>>
>>>> Now, if we were to change the paradigm so that the Widget class
>>>> establishes and releases the lock, and that the user should never handle
>>>> that, then that might be a partial solution, but still leaves unsolved the
>>>> user's cleanup needs.
>>>>
>>>> Ben Root
>>>>
>>>> I just started looking at the Lasso widget for some other changes, and
>>> I agree: the `points > 2` check should be removed.
>>>
>>> As for the design of Lasso, I don't quite understand why it disconnects
>>> the callbacks after a single call. If the onpress event from the lasso
>>> demo<http://matplotlib.sourceforge.net/examples/event_handling/lasso_demo.html>was made a method of Lasso, then you could reuse the Lasso widget instead
>>> of having to create a new one for each selection. Of course, this might
>>> complicate the widget lock used in the demo, but that's true for all other
>>> widgets, right?
>>>
>>> -Tony
>>>
>>>
>> The Lasso disconnects itself after the button_release event because
>> that's what indicates that you are done.  The user gets back a single
>> Line2D object that is assumed to represent a single path with no breaks.
>> Reusing the Lasso widget would be a situation that would require a
>> different idiom for user interaction.  I wouldn't be against a "MultiLasso"
>> widget that works differently from Lasso, but I really wouldn't want to
>> make changes to existing user widgets.  It is iffy enough about whether to
>> remove the point-count-check.
>>
>> Ben Root
>>
>>
> Wouldn't this argument suggest that `RectangleSelector` and `SpanSelector`
> should disconnect as well? I think their functionalities and their
> behaviors are (or should be) pretty similar.
>

Hadn't thought about that, and I don't use them, so I hadn't noticed.


>
> In any case, I agree that changing this behavior of `Lasso` is not
> desirable for compatibility reasons. I'll write up something that looks
> more like `RectangleSelector` and `SpanSelector`. Maybe `LassoSelector`?
>

Actually, I like that idea.  It gives a consistent naming scheme.

Ben Root