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

[Tony Yu <ts...@gm...>]

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