GeezerVison is showing github.com — [Bug]: FuncAnimation function not typed properly · Issue #29960 · matplotlib/matplotlib
[Bug]: FuncAnimation function not typed properly #29960DescriptionBug summary
Type checkers complain when one uses a function that returns None as the parameter of FuncAnimation. However, when blit=False, the return value of the function is ignored.
Code for reproduction
import matplotlib.pyplot as plt
from matplotlib import rc
from matplotlib.animation import FuncAnimation
rc("animation", html="jshtml")
fig, ax = plt.subplots()
def update(frame: int) -> None:
ax.plot([0, 1], [frame, frame])
FuncAnimation(fig, update, range(10))Actual outcome
MyPy says Argument 2 to "FuncAnimation" has incompatible type "Callable[[int], None]"; expected "Callable[..., Iterable[Artist]]", but the code executes without problems.
Expected outcome
No MyPy error.
Additional information
I think is possible to allow functions that return arbitrary object by combining overload and Literal[True] / Literal[False] for the blit argument.
Operating system
Any
Matplotlib Version
3.10.0
Matplotlib Backend
No response
Python version
No response
Jupyter version
No response
Installation
pip Activitytacaswell commented on Apr 27, 2025 I am against making any change here. The documented function signature is to return a list of artists that have been updated and in the case of not blitting we currently do not leverage that does not mean we should accept callbacks with the incorrect signature.
As a side note, that callback is a bit counter to the design of FuncAnimation which is built around the idea that you are updating existing artists not adding new artists at each step (about once a year we get a bug report "animation slows down with many frames" when the user is adding a new image every step through and then on each frame we draw all of them resulting in a line-painter bug). I hope that the mypy warning here will nudge users to use the API as intended. vnmabus commented on Apr 27, 2025 ContributorAuthor More actions I would say that this is a very widespread use-case. Even in your examples you have at least one case where nothing is returned in your update function:
https://matplotlib.org/stable/gallery/animation/rain.html#sphx-glr-gallery-animation-rain-py
To be fair, when I encountered this error I tried to change the update functions in the way you suggested, and I was able to improve some of them. However for some artists the contortions and the repetition of code that are needed to modify them vs creating a new one, make that a difficult and unreadable choice. vnmabus commented on Apr 27, 2025 ContributorAuthor More actions tacaswell commented on Apr 28, 2025 I've opened a PR to fix that example!
I do not like the path of having a "valid" callback which is blessed by static analysis that is then rendered invalid by changing a kwarg. My understanding of the type analyzers is that they will miss these overrides when blit is set via keyword / **kwargs if creation of FuncAnimation is being wrapped.
I also have some concerns that if we document via typing that we do not use the returned artists then we can not chose to start using them without a deprecation cycle (the counter argument to that is we would already be careful about starting to use them because, as you correctly note, there is a lot of user code floating around that does not return the artists). vnmabus commented on Apr 28, 2025 ContributorAuthor More actions
I do not like the path of having a "valid" callback which is blessed by static analysis that is then rendered invalid by changing a kwarg. My understanding of the type analyzers is that they will miss these overrides when blit is set via keyword / **kwargs if creation of FuncAnimation is being wrapped.
You have a point.
I also have some concerns that if we document via typing that we do not use the returned artists then we can not chose to start using them without a deprecation cycle (the counter argument to that is we would already be careful about starting to use them because, as you correctly note, there is a lot of user code floating around that does not return the artists).
Even if they currently returned the artists, if they do not pass blit that has not been checked, so they could have missed to return some artists and the behavior would change. I would assume that it is not possible to start using or even validating the returned object without a deprecation cycle. I still think that it would be convenient (and agrees with current usage) to accept a None return, and consider that equivalent to the sequence of all the artists in the figure. timhoffm commented on Apr 28, 2025 I've always felt it a bit cluncky that the user has to return the modified artists. Alternative ideas:
- I don't claim to understand all details, but can the list be individual per frame? I thought with blitting you have to decide what's on the background, and what will be rendered dynamically on top. That would mean the list has to be the same every time and then it would make more sense to have it stated once (e.g.
FuncAnimation(..., blit=True, blitted_artists=[...]) or take from the return of init_func, which has admittedly the same signature issue).
- Can't we be smarter and automatically determine which artists have been modified so that the user does not have to specify the modified artists manually?
timhoffm commented on Apr 28, 2025
accept a None return, and consider that equivalent to the sequence of all the artists in the figure.
Bit that would counter the purpose of blitting? vnmabus commented on Apr 28, 2025 ContributorAuthor More actions
Bit that would counter the purpose of blitting?
Yes, but it would make that signature acceptable when blit=False. rcomer commented on Apr 28, 2025 rcomer commented on Apr 28, 2025
- I don't claim to understand all details, but can the list be individual per frame?
Here's an example where we swap which artist is returned
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation
import numpy as np
fig, ax = plt.subplots()
ax.set_ylim(0, 10)
ax.set_ylabel('Points')
bar_container = ax.bar(['Team 1', 'Team 2'], [0, 0])
rng = np.random.default_rng()
def update(frame):
win_index = rng.integers(0, 2)
winner = bar_container.patches[win_index]
winner.set_height(winner.get_height() + 1)
return [winner]
anim = FuncAnimation(fig, update, 20, blit=True)
anim.save('bar_race.gif')
However, if I plt.show() instead of saving, I only see one bar at a time (with either TkAgg or QtAgg).
Edit: am also confused why the bars don't start from zero - this happens regardless of whether I return one or both of them 😕 timhoffm commented on Apr 28, 2025
- I don't claim to understand all details, but can the list be individual per frame?
Here's an example where we swap which artist is returned
However, if I plt.show() instead of saving, I only see one bar at a time (with either TkAgg or QtAgg).
It think we have to dig deeper, what's happening inside.
|
for a in self._drawn_artists: |
|
a.set_animated(self._blit) |
self._blit is True here, so it's effectively a.set_animated(True). I'm not aware that this get's reset anywhere, so it seems like we're collecting all animated artists over time. (Which I have doubts that it is reasonable).
Edit: am also confused why the bars don't start from zero - this happens regardless of whether I return one or both of them 😕
That's likely #29856 (comment). timhoffm commented on Apr 28, 2025 Just as expected, adding print(" ".join(str(p.get_animated()) for p in bar_container.patches)) to your update() function yields:
False False
False True
False True
True True
True True
True True
True True
...
So yes, once the artists are registered, they stay animated. Which makes sense because blitting creates a static background and you cannot selectively change the content of that background. Each artist has to be either part of the background or it must be rendered on every frame.
A backwards compatible migration strategy could be:
- If
init_func is given, store it's returned artists as drawn_artists - This makes it clear that the drawing is only decided once.
- Accept
func to return list[Artist] | None. Meaning add the given artists to the drawn_artists - this is incremental not absolute as before. This is needed (a) for backward compatibility and (b) in case you want to create new artists in func.
- Add a new parameter
drawn_artists (name t.b.d.) for FuncAnimation, equivalent to the return value of init_func for the case that no init_func is used.
This should cover all cases:
- The above with an outside-created figure state and explicit func return. Here
drawn_artists is empty on init and will be populated after the first draw.
- The above with an outside-created figure state, no func return, but a new
FuncAnimation(..., drawn_artists=bar_container.patches). Here drawn_artists is empty on init and will be populated after the first draw.
- The above with an init_func and explicit func return. Here
drawn_artists will be populated through the init_func and update through func (though the update has no effect as the artists are already in there.
This feels slightly more sane.
Additional remarks:
- We technically can run though the artist tree before and after func and detect whether
func has changed the stale state of the artists. This could in principle auto-detect the blitted artists. However, without fully understanding the logic, I assume there are some subtleties making this harder in practice (e.g. drawing could make the Axes/figure stale, we may need the drawn artists up front to draw the background). They could be solvable but it's likely a bit of work.
- On are more general note, I don't find the current pattern of using an existing figure very clean. The
udpate() func must use implicit external state (global or enclosed) to reach for the artists it wants to modify. This is part of the reason it's hard to define/track which artists are modified.
Edit: If we want to keep it small here, we could also do it the other way round: define func(...) -> list[Artist] | None where None is equivalent to an empty list. Then raise a runtime error if blit=True and func returns None. That way type checkers do not issue false positives, at the expense of not flagging the needed return for blitting. But maybe better that way than complaining about actually working code. See #29984. added 2 commits that reference this issue on Apr 28, 2025This issue will close once commit 1bd5b15 is merged into the 'main' branch.1bd5b15 This issue will close once commit db0ec14 is merged into the 'main' branch.db0ec14
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
