Event handling broken in svn?

Howdy,

I spent a while chasing my tail today with some event handling code
until I tried backtracking from SVN matplotlib back to 0.99.1 (the one
in ubuntu 10.04) and the problem went away.

I'm attaching a script that reproduces the problem with a full
description in the docstring, reproduced here for completeness:

This example wires two event handlers that both respond to clicks by printing
event info identically. One is written as a standalone function, the other as
a method of an object.

- when run with MPL 0.99.1.1 (stock in ubuntu 10.04), both fire fine:

amirbar[mplbrush]> python mpleventbug.py
MPL version: 0.99.1.1
MPL: /usr/lib/pymodules/python2.6/matplotlib/__init__.py
/usr/lib/pymodules/python2.6/matplotlib/backends/backend_gtk.py:621:
DeprecationWarning: Use the new widget gtk.Tooltip
  self.tooltips = gtk.Tooltips()
C1 - button=1, x=146, y=229, xdata=23.783488, ydata=0.846491
C2 - button=1, x=146, y=229, xdata=23.783488, ydata=0.846491
C1 - button=1, x=216, y=189, xdata=42.919628, ydata=0.671053
C2 - button=1, x=216, y=189, xdata=42.919628, ydata=0.671053
C1 - button=1, x=288, y=117, xdata=62.602515, ydata=0.355263
C2 - button=1, x=288, y=117, xdata=62.602515, ydata=0.355263
etc...

- when run with matplotlib r8721, the one that is a method does not fire:

amirbar[mplbrush]> python mpleventbug.py
MPL version: 1.0.0
MPL: /home/fperez/usr/opt/lib/python2.6/site-packages/matplotlib/__init__.pyc
C1 - button=1, x=150, y=170, xdata=24.876982, ydata=0.587719
C1 - button=1, x=169, y=160, xdata=30.071077, ydata=0.543860
C1 - button=1, x=187, y=135, xdata=34.991799, ydata=0.434211
C1 - button=1, x=210, y=120, xdata=41.279388, ydata=0.368421

mpleventbug.py (2.25 KB)

···

###

This manifested itself in some more complex MPL code that had multiple
events not working when run inside ipython, but working OK outside of
ipython. Fortunately, the small self-contained example demonstrates
the problem even with ipython not being in the picture at all (the
runs above were from the command line), so I think there is an issue
in MPL proper.

Sorry that I can't dig deeper into the code right now to look for a fix...

Timing note: EPD is planning a release in a few weeks, I don't know
how close MPL is to a bugfix release in the 1.0.x series. I don't
know what version of mpl EPD plans to use, but if event handling is
really broken and a fix is feasible in the time available, it might be
worth pushing it through.

Cheers,

f

Somewhere in the 1.0 development cycle, Michael modified the callback
code to take weak references to methods. The purpose was to eliminate
some "leaks" that were occurring because callback connections to
objects were keeping them around and the proper disconnects were not
made (much simpler fix than tracking down every mpl_connect and trying
to see where do disconnect). What you're seeing in your script is that
since you're not assigning the Handler object to anything, it's being
garbage collected. It works for me if I change the second to last line
to:

    h = Handler(f)

Ryan

···

On Fri, Oct 1, 2010 at 1:05 AM, Fernando Perez <fperez.net@...149...> wrote:

This manifested itself in some more complex MPL code that had multiple
events not working when run inside ipython, but working OK outside of
ipython. Fortunately, the small self-contained example demonstrates
the problem even with ipython not being in the picture at all (the
runs above were from the command line), so I think there is an issue
in MPL proper.

Sorry that I can't dig deeper into the code right now to look for a fix...

--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma

Hey Ryan,

This manifested itself in some more complex MPL code that had multiple
events not working when run inside ipython, but working OK outside of
ipython. Fortunately, the small self-contained example demonstrates
the problem even with ipython not being in the picture at all (the
runs above were from the command line), so I think there is an issue
in MPL proper.

Sorry that I can't dig deeper into the code right now to look for a fix...

Somewhere in the 1.0 development cycle, Michael modified the callback
code to take weak references to methods. The purpose was to eliminate
some "leaks" that were occurring because callback connections to
objects were keeping them around and the proper disconnects were not
made (much simpler fix than tracking down every mpl_connect and trying
to see where do disconnect). What you're seeing in your script is that
since you're not assigning the Handler object to anything, it's being
garbage collected. It works for me if I change the second to last line
to:

h = Handler(f)

Many thanks for the info, that helps a lot.

I was wondering though, would we still have a leak if strong
references are held in the canvas attribute? The canvas will be
deleted when the figure goes away, so that should properly allow the
callback references to be deleted, without deleting them early
otherwise, no?

In any case, if my logic is flawed (quite likely, since I imagine M.
D. had a good look at this), it might be worth adding a

.. warning::

section about this pattern to the event docs:

http://matplotlib.sourceforge.net/users/event_handling.html

because the problem is subtle and hard to diagnose (I just noticed it
had also been reported recently
http://sourceforge.net/mailarchive/forum.php?thread_name=4C9B7793.5020908%40gmail.com&forum_name=matplotlib-devel).

In any case, thanks again for the help!

Cheers,

f

···

On Fri, Oct 1, 2010 at 6:27 AM, Ryan May <rmay31@...149...> wrote:

On Fri, Oct 1, 2010 at 1:05 AM, Fernando Perez <fperez.net@...149...> wrote:

Done in 8723.

Eric

···

On 10/01/2010 08:01 AM, Fernando Perez wrote:

In any case, if my logic is flawed (quite likely, since I imagine M.
D. had a good look at this), it might be worth adding a

.. warning::

section about this pattern to the event docs:

http://matplotlib.sourceforge.net/users/event_handling.html

Thanks!

Cheers,

f

···

On Sun, Oct 3, 2010 at 2:05 PM, Eric Firing <efiring@...229...> wrote:

section about this pattern to the event docs:

http://matplotlib.sourceforge.net/users/event_handling.html

Done in 8723.

Hey Ryan,

This manifested itself in some more complex MPL code that had multiple
events not working when run inside ipython, but working OK outside of
ipython. Fortunately, the small self-contained example demonstrates
the problem even with ipython not being in the picture at all (the
runs above were from the command line), so I think there is an issue
in MPL proper.

Sorry that I can't dig deeper into the code right now to look for a fix...
       

Somewhere in the 1.0 development cycle, Michael modified the callback
code to take weak references to methods. The purpose was to eliminate
some "leaks" that were occurring because callback connections to
objects were keeping them around and the proper disconnects were not
made (much simpler fix than tracking down every mpl_connect and trying
to see where do disconnect). What you're seeing in your script is that
since you're not assigning the Handler object to anything, it's being
garbage collected. It works for me if I change the second to last line
to:

    h = Handler(f)
     

Many thanks for the info, that helps a lot.

I was wondering though, would we still have a leak if strong
references are held in the canvas attribute? The canvas will be
deleted when the figure goes away, so that should properly allow the
callback references to be deleted, without deleting them early
otherwise, no?
   

The problem is when callbacks create cyclical references (which your example does not). If the Handler class in your example needed to update the figure or canvas in some way in the callback (which is a common usage pattern), that cyclical reference prevents either from being destroyed without running the cyclical garbage collector. And in that case, you can't write a __del__ method on the handler to explicitly disconnect the callbacks.

In any case, if my logic is flawed (quite likely, since I imagine M.
D. had a good look at this), it might be worth adding a

.. warning::

section about this pattern to the event docs:

http://matplotlib.sourceforge.net/users/event_handling.html

because the problem is subtle and hard to diagnose (I just noticed it
had also been reported recently
http://sourceforge.net/mailarchive/forum.php?thread_name=4C9B7793.5020908%40gmail.com&forum_name=matplotlib-devel).
   

True -- it's non-obvious and confusing. On the other hand, the user is no longer required to explicitly disconnect callbacks, which was the source of many other subtle and hard to diagnose problems within the matplotlib code itself.

I'm still not completely happy with it, so I'd love to find a "third way" if there's anything anyone can suggest.

Mike

···

On 10/01/2010 02:01 PM, Fernando Perez wrote:

On Fri, Oct 1, 2010 at 6:27 AM, Ryan May<rmay31@...149...> wrote:

On Fri, Oct 1, 2010 at 1:05 AM, Fernando Perez<fperez.net@...149...> wrote:

--
Michael Droettboom
Science Software Branch
Space Telescope Science Institute
Baltimore, Maryland, USA

Thanks for your explanation, it makes complete sense.

I think it's OK, Eric just added a warning to the docs, which will go
a long ways towards making this less of a user trap. Given the
details you provided, I can't think of a generic way to handle these
cycles 100% automatically. Using weakrefs seems like the most
sensible solution, and users will just need to understand a little bit
more before using this functionality.

Event handling isn't raw beginner material in any case, so I don't
think it's a huge problem. And if someone ever devises a clever
solution to the problem, then great! But for now I think you can
safely ignore this further.

Regards,

f

···

On Mon, Oct 4, 2010 at 6:13 AM, Michael Droettboom <mdroe@...31...> wrote:

The problem is when callbacks create cyclical references (which your
example does not). If the Handler class in your example needed to
update the figure or canvas in some way in the callback (which is a
common usage pattern), that cyclical reference prevents either from
being destroyed without running the cyclical garbage collector. And in
that case, you can't write a __del__ method on the handler to explicitly
disconnect the callbacks.

In any case, if my logic is flawed (quite likely, since I imagine M.
D. had a good look at this), it might be worth adding a

.. warning::

section about this pattern to the event docs:

http://matplotlib.sourceforge.net/users/event_handling.html

because the problem is subtle and hard to diagnose (I just noticed it
had also been reported recently
http://sourceforge.net/mailarchive/forum.php?thread_name=4C9B7793.5020908%40gmail.com&forum_name=matplotlib-devel).

True -- it's non-obvious and confusing. On the other hand, the user is
no longer required to explicitly disconnect callbacks, which was the
source of many other subtle and hard to diagnose problems within the
matplotlib code itself.

I'm still not completely happy with it, so I'd love to find a "third
way" if there's anything anyone can suggest.