matplotlib-devel Mailing List for matplotlib

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

On Tue, Feb 28, 2012 at 10:58 AM, John Hunter <jd...@gm...> wrote:
>
>
> On Tue, Feb 28, 2012 at 10:44 AM, Benjamin Root <ben...@ou...> wrote:
>
>>
>> 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.
>>
>>
> I added the point count check according to git blame and I don't remember
> why but I agree is doesn't look like it makes sense to me. I think this
> lasso is lightly used based on the volume of questions we get about it.
> One reason it may be lightly used is because it is hard to work with. If
> we can improve it, even changing functionality, I would be in favor. Eg,
> allowing the same Lasso to handle repeated selections makes sense to me.
> If the change looks too onerous, we could call it Lasso2 and deprecate the
> former.
>
I think that logic is slightly faulty. Another reason why we don't get a
lot of questions about it could be because it does (almost) exactly what
users want. Currently, I use the Lasso widget in my track_maker project to
select storm cells in a movie of radar images. Because I have multiple
modes to the program, a persistent Lasso object makes little sense. I also
want the Lasso's line to disappear after I am done selecting it so that I
can put in a polygon patch of my coloring and hatching.
I am skeptical of any redesign of Lasso. I am willing to see what others
have in mind, but I still think that it might better belong in a new class
"MultiLasso". I see no reason to deprecate the current Lasso (only to fix
it).
Cheers!
Ben Root

On Tue, Feb 28, 2012 at 10:44 AM, Benjamin Root <ben...@ou...> wrote:
>
> 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.
>
>
I added the point count check according to git blame and I don't remember
why but I agree is doesn't look like it makes sense to me. I think this
lasso is lightly used based on the volume of questions we get about it.
 One reason it may be lightly used is because it is hard to work with. If
we can improve it, even changing functionality, I would be in favor. Eg,
allowing the same Lasso to handle repeated selections makes sense to me.
 If the change looks too onerous, we could call it Lasso2 and deprecate the
former.

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

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