Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timers independent of canvases #1907

Closed
wants to merge 8 commits into from
Closed

Conversation

mdehoon
Copy link
Contributor

@mdehoon mdehoon commented Apr 15, 2013

The animation code in matplotlib relies on timers to update the animated figures. Currently a new timer is created by calling new_timer on a canvas, as in

>>> f = pylab.figure()
>>> timer = f.canvas.new_timer()

This seems a bit of a wrinkle. For example, you may want to associate a timer with multiple figures, or with no figure at all; also you may want to continue using a timer after a particular figure is closed.
I am therefore proposing to make timers independent of canvases. With this pull request, the above code would become

>>> from matplotlib import events
>>> timer = events.Timer()

This has the additional advantage of making the different backends more similar to each other; in the current implementation some backends rely on the canvas when making a timer, while others ignore it.
I have verified that the animation examples still work correctly with all backends.

(EDIT @pelson: added code blocks)

@WeatherGod
Copy link
Member

I will try this out today or tomorrow with my more "complicated" animations
that I have for my dissertation.

On Mon, Apr 15, 2013 at 6:00 PM, mdehoon notifications@github.com wrote:

The animation code in matplotlib relies on timers to update the animated
figures. Currently a new timer is created by calling new_timer on a canvas,
as in

f = pylab.figure()
timer = f.canvas.new_timer()
This seems a bit of a wrinkle. For example, you may want to associate a
timer with multiple figures, or with no figure at all; also you may want to
continue using a timer after a particular figure is closed.
I am therefore proposing to make timers independent of canvases. With this
pull request, the above code would become
from matplotlib import events
timer = events.Timer()
This has the additional advantage of making the different backends more
similar to each other; in the current implementation some backends rely on
the canvas when making a timer, while others ignore it.
I have verified that the animation examples still work correctly with all
backends.


You can merge this Pull Request by running

git pull https://github.com/mdehoon/matplotlib Timer

Or view, comment on, or merge it at:

#1907
Commit Summary

  • Making the Tkagg Timer independent of the canvas
  • Making the WX timer independent of the canvas
  • parent is not needed any more
  • Putting the Timer code in a separate submodule (events)
  • Keep the docstring in only one place
  • make sure the backend base timer is always available
  • provide _timer for gtk3
  • Removing the now obsolete new_timer method

File Changes

Patch Links:

@pelson
Copy link
Member

pelson commented Apr 17, 2013

I really like the concept of having a matplotlib.events.Timer class. Unfortunately, because the timer implementations are backend dependent, matplotlib.events is bound to have to call matplotlib.get_backend() which itself is bound to using the rcParams. All of this means that it is no longer posible to use the full OO interface without having to set the rcParams "backend" to the appropriate value.

Currently a new timer is created by calling new_timer on a canvas. This seems a bit of a wrinkle. For example, you may want to associate a timer with multiple figures, or with no figure at all.

Agreed - this sounds very desirable to me. The big problem, IMHO, is that there is no figure-less concept to be initialised in a backend module (FigureCanvases are bound to a single figure instance, and FigureManagers are bound to a single canvas) which should be responsible for initialising the Timer.

Interestingly the TkAgg backend's timer requires a Tk.Canvas instance - so without creating one (perhaps via a FigureCanvasTkAgg instance) - you will not be able to have a timer without a figure anyway.

All of this suggests to me that we are missing a backend concept (but I'm not 100% sure what that is), which is responsible for managing backend specific stuff such as Timers (and perhaps the "show" event loop).

Anyone who uses the real OO interface for GUI backends care to comment (is there anybody!?!)?

@mdehoon
Copy link
Contributor Author

mdehoon commented Apr 17, 2013

In my branch I am indeed using matplotlib.get_backend in matplotlib.events. Perhaps an alternative is to copy the _backend_selection function from matplotlib/pyplot.py (or put it in a more central place)? Because as far as I can tell, matplotlib.events and matplotlib.pyplot are facing the same problem with regards to identifying the backend.

The big problem, IMHO, is that there is no figure-less concept to be initialised in a backend module
(FigureCanvases are bound to a single figure instance, and FigureManagers are bound to a single canvas)
which should be responsible for initialising the Timer.

There is a Timer class in the backend modules, which is defined independently of any figure. In my branch, in each backend a Timer can be initialized without needing a figure.

Interestingly the TkAgg backend's timer requires a Tk.Canvas instance - so without creating one (perhaps via a
FigureCanvasTkAgg instance) - you will not be able to have a timer without a figure anyway.

While the Timer in TkAgg did require a Tk.Canvas instance, it is possible to make a Timer in Tkinter without using a Canvas. See the Timer class in TkAgg in my branch:

https://github.com/mdehoon/matplotlib/blob/Timer/lib/matplotlib/backends/backend_tkagg.py

All of this suggests to me that we are missing a backend concept (but I'm not 100% sure what that is), which is
responsible for managing backend specific stuff such as Timers (and perhaps the "show" event loop).

I guess we could make importing the backend a bit easier by centralizing the backend selection code. It would be great if we could just do

from matplotlib import backend

@mdboom
Copy link
Member

mdboom commented Apr 17, 2013

👍 to from matplotlib import backend if we can make that work.

@pelson
Copy link
Member

pelson commented Apr 17, 2013

from matplotlib import backend

I feel like the bad parent 😄, but any solution which makes use of the rcParams['backend'] value is, by definition, breaking the ability to use of the OO interface without setting the appropriate rcParam. Is there a reason we need the shortened interface for this? Can we not simply add a function (or class method to Timer) to each backend which supports timers and use that? Are there any interfaces which should not know about which backend they are dealing with?

I think this functionality is really valuable - but I'd also like to find a solution which would allow the OO interface to continue to function (without the use of globals, such as rcParams), so I'm keen to help out in any way I can.

Cheers,

@mdboom
Copy link
Member

mdboom commented Apr 17, 2013

Sorry, I should have been more clear. While from matplotlib import backend would be handy for a lot of reasons, we obviously can't rely on that as the only method to get a timer. Each backend will also need a consistent way to ge ta timer, and the user can either use matplotlib.backend or matplotlib.backends.some_specific_backend to get to it, depending on whether their code cares about what specific backend is being used.

So I don't think what @pelson is saying is really in conflict with what I'm saying, unless I'm missing something.

@WeatherGod
Copy link
Member

Just to note, I did not notice any adverse effects with my more complicated animations, both in displaying, and saving them.

@mdehoon
Copy link
Contributor Author

mdehoon commented Apr 17, 2013

@pelson, @mdboom Sorry I am not sure if I understand correctly.
If we can use

from matplotlib import backend
timer = backend.Timer(interval=1000, callbacks=my_callbacks)
timer.start()

and

from matplotlib.backends import tkagg
timer = tkagg.Timer(interval=1000, callbacks=my_callbacks)
timer.start()

and

import Tkinter
tcl = Tkinter.Tcl()
timer = tcl.after(interval=1000, callback=my_callback)

then what is still missing?

backend_module = __import__(backend_name, globals(), locals(), [backend_name])
return backend_module

backend = _initialize()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this is a one-time event & bound to rcParams['backend']. Users who create import and create their own Canvas from a backend module (which is not the rcParams backend) can therefore not use this module, and ergo cannot use animations.

@pelson
Copy link
Member

pelson commented Apr 18, 2013

@mdehoon - those options are great (in fact, all we need). The problem is that the first is relying on the rcParams['backend'] global to initialize the timer where before one could initialize the timer without using the rcParam.

I don't actually use the full OO interface, but I've put together an example of what used to work before this PR:

import matplotlib
# this is just set to throw any globals off the scent of me using TkAgg...
matplotlib.use('Agg')


import time
import matplotlib.backends.backend_tkagg as backend
from matplotlib.animation import FuncAnimation
import numpy as np


def update_line(num, line):
    t = time.time()
    x = np.linspace(0, np.pi / 4., 1000) + t
    line.set_data(np.sin(x), np.sin(x) * np.cos(x))


manager = backend.new_figure_manager(0)
fig = manager.canvas.figure
ax = fig.add_subplot(1, 1, 1)
line, = ax.plot([-1, 1], [-1, 1])
update_line(0, line)
FuncAnimation(fig, update_line, fargs=(line, ), interval=10)._start()
fig.show()
backend.show.mainloop()

I'm not confident that this is the best way to use the OO interface, but it is an example which does fail as a result of matplotlib.events depending on rcParams.

HTH

@mdehoon
Copy link
Contributor Author

mdehoon commented Apr 18, 2013

@pelson Thanks for the example. I see your point.

Then, I guess the fundamental question is, how does matplotlib know which backend is to be used?
Importing a specific backend module directly can make this tricky, but how about the following:

  • matplotlib.backend is the current backend module;
  • matplotlib.backend is initialized to what is specified in rcParams, if available;
  • users can select a different backend by matplotlib.use;
  • matplotlib.use updates the matplotlib.backend variable

Then, in your example, we'd get

import matplotlib
matplotlib.use('TkAgg')
...
backend = matplotlib.backend
manager = backend.new_figure_manager(0)
...

and inside animations.py, instead of

from matplotlib import events
timer = events.Timer()

we'd have

from matplotlib import backend
timer = backend.Timer()

The events module can then be removed.

@mdboom
Copy link
Member

mdboom commented Apr 18, 2013

As long as

from matplotlib.backends import backend_tkagg
timer = backend_tkagg.Timer()

also works, I think that's fine. There is a lot of code out there written to a specific backend (for good reason), and that code doesn't even necessary call use or set the backend rcParam.

@mdehoon
Copy link
Contributor Author

mdehoon commented Apr 18, 2013

Yes that would also still work.

@WeatherGod
Copy link
Member

My only concern there is that we need to watch out for dependency chaining
(now and in the future). If someone imports something in that indirectly
imports the backend, and someone uses matplotlib.use() later (but before
any figures are created) are those imports of backend still valid? Also, I
am not a big fan of having "matplotlib.backends" as an actual module and a
"matplotlib.backend" as some sort of meta-module. The one-letter
difference is going to confuse the heck out of people, I think.

On Thu, Apr 18, 2013 at 6:41 PM, mdehoon notifications@github.com wrote:

Yes that would also still work.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1907#issuecomment-16616810
.

@mdboom
Copy link
Member

mdboom commented May 20, 2013

I'd like to move this forward for 1.3. @WeatherGod: Any thoughts on how to address your concerns and still get the meat of this done (which will allow timers to work in the osx backend)?

@@ -16,7 +17,7 @@ def update_title(axes):

# Create a new timer object. Set the interval 500 milliseconds (1000 is default)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is different from what is implemented

@WeatherGod
Copy link
Member

My big concern is with _initialize() in events.py. If we can update the code that does the backend switching to also handle switching of timer modules, that would make me feel better.

But there is still something not sitting right with me here. First off, we are completely throwing out TimerGTK and eliminating functions like "new_timer()" and such without a deprecation process. Remember, it was the animation module that was labeled as "experimental" not the Timers, which have been in place for a few releases now. I am sure we can very easily have "TimerGTK" aliases and new_timer().

@mdehoon
Copy link
Contributor Author

mdehoon commented May 21, 2013

I also think that this is not quite ready yet. At least the PR will need to be updated to reflect the outcome of the discussion here.

One thing that is not quite clear to me is to what degree we allow users to import specific backends directly. This can work in specific cases, but not in general even with matplotlib 1.2.1. For example, in this code snippet:

import matplotlib
matplotlib.use("tkagg")
matplotlib.interactive(False)
from pylab import *

figure()
from matplotlib.backends import backend_gtk

def f():
    print 'test'

timer = backend_gtk.TimerGTK(1000, ((f,[],{}),))
timer.start()
show()

the timer is not being called because the Tkinter event loop is unaware of the GTK timer.

So I think we do need some centralized way of selecting the backend (while importing specific backends directly may or may not work).

@pelson
Copy link
Member

pelson commented May 21, 2013

Given what's been previously said, I'd like to move this out of the v1.3.x milestone. There is a lot of delicate code which we would not be ideal to change right before a release. @mdboom - you put it in v1.3.x, what do you think?

@mdboom
Copy link
Member

mdboom commented May 21, 2013

Yes -- I was thinking the same thing. This is very worthwhile, but best to put it off to do it right.

@pelson
Copy link
Member

pelson commented Jan 9, 2014

We've stalled on this PR. It is a really gnarly issue - is it worth having a hangout about this subject? @mdehoon - would you be able to make a Thursday (last one's times in your TZ: https://www.google.com/calendar/render?eid=OHRndGhhYnZnYW1wMG1iaWtvNDhuMmxlZWsgNzloazhqaHZsa3M4am44ZHM0cmkxZTZxNGdAZw&ctz=America/New_York&pli=1&t=AKUaPmaSkt5RzvGkQLNITYToz4oXol9K5S9VJLt625fiwb-JfRrrfS9-Y2lXNFrXHWqH4Z47yqhA&sf=true&output=xml).

In the meantime, I don't think we can merge this and will definitely need a MEP to get a consensus on the ambition. As a result, I'm going to close the PR, if only to keep the number of PRs as low as is feasible.

Cheers @mdehoon

@pelson pelson closed this Jan 9, 2014
@mdehoon
Copy link
Contributor Author

mdehoon commented Jan 10, 2014

@pelson I can do a hangout to discuss, but it would have to be outside of working hours in the Japanese time zone (the previous one was at 10:00-12:00 in the morning here, so that wouldn't work). As it happens, this coming Monday January 13th is a holiday in Japan, so that would work.

@pelson
Copy link
Member

pelson commented Jan 10, 2014

@mdboom, @WeatherGod - is that any good for you. Same time as normal, just this Monday instead? If so, lets put out a notice on the mailinglist today. I suggest we keep the subjects to 2: First the timer discussion; second release related business/PRs.

@mdboom - can you grant me access to the matplotlib group calendar so that I can add the proposed date?

Cheers,

@mdboom
Copy link
Member

mdboom commented Jan 13, 2014

@pelson: Sorry to not see this until now. I suspect it's too late to have this meeting at this point, but in general, I can do most Mondays at that time. I've added you as a calendar manager -- I guess you'll get a notification of that through your gmail address...?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants