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.

Sign up for GitHub

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

Jump to bottom

address FuncAnimantion trying to take lengths of generators #2634

Merged
merged 4 commits into from Jan 6, 2014
Merged

address FuncAnimantion trying to take lengths of generators #2634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b57c6dc
Check if iterator has length
tacaswell Nov 30, 2013
03c71f5
added test for code path where the object passed into `frames` is
tacaswell Dec 12, 2013
8ab1f0a
corrected test function name
tacaswell Dec 12, 2013
1b144e8
added test_animation back in to default_test_modules so that the
tacaswell Dec 12, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/matplotlib/__init__.py
View file
Open in desktop
Expand Up @@ -1288,6 +1288,7 @@ def tk_window_focus():

default_test_modules = [
'matplotlib.tests.test_agg',
'matplotlib.tests.test_animation',
'matplotlib.tests.test_arrow_patches',
'matplotlib.tests.test_artist',
'matplotlib.tests.test_axes',
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/animation.py
View file
Open in desktop
Expand Up @@ -1000,7 +1000,8 @@ def __init__(self, fig, func, frames=None, init_func=None, fargs=None,
self._iter_gen = frames
elif iterable(frames):
self._iter_gen = lambda: iter(frames)
self.save_count = len(frames)
if hasattr(frames, '__len__'):
self.save_count = len(frames)
Copy link
Member

Choose a reason for hiding this comment

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

What happens if save_count isn't set. Is this a problem?

Copy link
Member

Choose a reason for hiding this comment

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

Oh. Urgh, it defaults to 100. Perhaps we are better assuming that the generator isn't infinite and casting it to a list?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see why that is a problem, it will just keep a rolling buffer of that length. It isn't useful, but it's not really harmful. I might also be confused.

I think it is important to keep the ability to have infinite generators, this is a quick way to spin up realtime-ish diagnostic displays.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, thanks @tacaswell - I misunderstood what thee save count was doing. Ignore me 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

That said, I have never figured out where the saved frames are used....

I am not sure if it is precursor work for a feature that never got finished or I just don't understand what it is doing.

Copy link
Member

Choose a reason for hiding this comment

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

@tacaswell - I think we should try to add a test case (hopefully it will be quite straight forward) which exercises this code path.

else:
self._iter_gen = lambda: xrange(frames).__iter__()
self.save_count = frames
Expand Down
19 changes: 19 additions & 0 deletions lib/matplotlib/tests/test_animation.py
View file
Open in desktop
Expand Up @@ -52,6 +52,25 @@ def animate(i):
anim.save(F.name, fps=30, writer=writer)


@cleanup
def test_no_length_frames():
fig, ax = plt.subplots()
line, = ax.plot([], [])

def init():
line.set_data([], [])
return line,

def animate(i):
x = np.linspace(0, 10, 100)
y = np.sin(x + i)
line.set_data(x, y)
return line,

anim = animation.FuncAnimation(fig, animate, init_func=init,
frames=iter(range(5)))

Copy link
Member

Choose a reason for hiding this comment

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

Should there be an assertion here? I appreciate the fact that you're calling the constructor, and that before it was raising an Exception, but it'd be nice to test the attributes which you expect to have been set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Given the discussion above about the value it defaults to and my confusion of what this value actually does (does it make sense to keep a rolling buffer of an infinite generator?) I think it is better to just test 'does it blow up' here due to the high chance of what it defaults to changing in the (near) future.


if __name__ == "__main__":
import nose
nose.runmodule(argv=['-s', '--with-doctest'], exit=False)