8000 Prevent exception when running animation on Agg backend. by anntzer · Pull Request #13229 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Prevent exception when running animation on Agg backend. #13229

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
Merged
merged 1 commit into from
Jan 27, 2019
Merged

Prevent exception when running animation on Agg backend. #13229

merged 1 commit into from
Jan 27, 2019

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Jan 19, 2019

Currently, running e.g. the strip_chart example using the agg backend
(not a particularly interesting use case, but done in particular by the
doc build) causes a "AttributeError: 'FuncAnimation' object has no
attribute '_cache_frame_data'" exception.

This is fixed by moving the setting of the _cache_frame_data attribute
to before the super().__init__ call.

Also move the setting of _init_func next to the one of _func, which
seems to be a logical grouping.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

< 8000 div class="d-flex flex-auto">
@anntzer
Prevent exception when running animation on Agg backend.
1b81a08 
Currently, running e.g. the strip_chart example using the agg backend
(not a particularly interesting use case, but done in particular by the
doc build) causes a "AttributeError: 'FuncAnimation' object has no
attribute '_cache_frame_data'" exception.

This is fixed by moving the setting of the `_cache_frame_data` attribute
to before the `super().__init__` call.

Also move the setting of `_init_func` next to the one of `_func`, which
seems to be a logical grouping.
@ImportanceOfBeingErnest
Copy link
Member

How did the Attribute Error find its way into the code base if doc builds are part of the testing suite?

@anntzer
Copy link
Contributor Author
anntzer commented Jan 19, 2019

I think because it occurs in an asynchronous event handler (via CallbackRegistry.process).

@dstansby
Copy link
Member

If it's a problem in an example, is it possible to add a test for this?

@anntzer
Copy link
Contributor Author
anntzer commented Jan 19, 2019

May be nice, but I'm not volunteering for that (it's actually a bit of a pain to test (if my interpretation is correct) because the exception is not in the main thread).

@dstansby
Copy link
Member

Well but as part of a bug fix I feel that as a general rule we should be introducing a test that makes sure the bug doesn't occur again in the future.

@timhoffm
Copy link
Member

Generally, if we have to shift initialization of variables in __init__ the design is fragile and there may be a more fundamental problem.

The full traceback is

Traceback (most recent call last):
  File "/home/tim/dev/matplotlib/lib/matplotlib/cbook/__init__.py", line 213, in process
    func(*args, **kwargs)
  File "/home/tim/dev/matplotlib/lib/matplotlib/animation.py", line 954, in _start
    self._init_draw()
  File "/home/tim/dev/matplotlib/lib/matplotlib/animation.py", line 1701, in _init_draw
    self._draw_frame(next(self.new_frame_seq()))
  File "/home/tim/dev/matplotlib/lib/matplotlib/animation.py", line 1714, in _draw_frame
    if self._cache_frame_data:
AttributeError: 'FuncAnimation' object has no attribute '_cache_frame_data'

Apparently Animation._start get's called before the child class is completely __init__ed. If that's necessary by design super().__init__() should be the last statement in every Animation subclass.

@anntzer
Copy link
Contributor Author
anntzer commented Jan 19, 2019

@dstansby I do agree with your point as a general rule, but it does not change the fact that I don't plan to write a test for this PR (both because the use case is obscur(ish) and the test is a pain to write (if I interpreted it correctly)).

@dopplershift
Copy link
Contributor

I, for one, am shocked at the idea that code I wrote a decade ago could be considered fragile. 😈

I can't even get the example to crash with the Agg backend. I say that to agree with the idea that testing may be hard.

dopplershift
dopplershift approved these changes Jan 22, 2019
View reviewed changes
Copy link
Contributor
@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Unless someone can come up with a test that can consistently trigger this, I say we merge the fix and get on with our lives. Perfect being the enemy of good and all that.

@tacaswell tacaswell added this to the v3.1 milestone Jan 27, 2019
@tacaswell tacaswell merged commit 4e87b19 into matplotlib:master Jan 27, 2019
@anntzer anntzer deleted the animation_init branch January 27, 2019 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dopplershift dopplershift dopplershift approved these changes

Assignees
No one assigned
Labels
topic: animation
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@anntzer @ImportanceOfBeingErnest @dstansby @timhoffm @dopplershift @tacaswell
0