You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Bug]: FuncAnimation function not typed properly #29960
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.
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.
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.
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:
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.
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).
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.
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?
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 😕
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.
Activity
tacaswell commentedon 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
FuncAnimationwhich 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 commentedon Apr 27, 2025
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 commentedon Apr 27, 2025
Also, for those not getting the reference: https://www.joelonsoftware.com/2001/12/11/back-to-basics/
tacaswell commentedon 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
blitis set via keyword /**kwargsif creation ofFuncAnimationis 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 commentedon Apr 28, 2025
You have a point.
Even if they currently returned the artists, if they do not pass
blitthat 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 aNonereturn, and consider that equivalent to the sequence of all the artists in the figure.timhoffm commentedon Apr 28, 2025
I've always felt it a bit cluncky that the user has to return the modified artists. Alternative ideas:
FuncAnimation(..., blit=True, blitted_artists=[...])or take from the return ofinit_func, which has admittedly the same signature issue).timhoffm commentedon Apr 28, 2025
Bit that would counter the purpose of blitting?
vnmabus commentedon Apr 28, 2025
Yes, but it would make that signature acceptable when
blit=False.rcomer commentedon Apr 28, 2025
FWIW we already document in the docstring that
https://matplotlib.org/stable/api/_as_gen/matplotlib.animation.FuncAnimation.html#matplotlib.animation.FuncAnimation
rcomer commentedon Apr 28, 2025
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).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 commentedon Apr 28, 2025
It think we have to dig deeper, what's happening inside.
matplotlib/lib/matplotlib/animation.py
Lines 1806 to 1807 in 828ec6f
self._blitisTruehere, so it's effectivelya.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).That's likely #29856 (comment).
timhoffm commentedon Apr 28, 2025
Just as expected, adding
print(" ".join(str(p.get_animated()) for p in bar_container.patches))to yourupdate()function yields: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:
init_funcis given, store it's returned artists asdrawn_artists- This makes it clear that the drawing is only decided once.functo returnlist[Artist] | None. Meaning add the given artists to thedrawn_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 infunc.drawn_artists(name t.b.d.) forFuncAnimation, equivalent to the return value ofinit_funcfor the case that noinit_funcis used.This should cover all cases:
drawn_artistsis empty on init and will be populated after the first draw.FuncAnimation(..., drawn_artists=bar_container.patches). Heredrawn_artistsis empty on init and will be populated after the first draw.drawn_artistswill 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:
funchas 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.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] | Nonewhere None is equivalent to an empty list. Then raise a runtime error ifblit=Trueand 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.FIX: Typing of FuncAnimation
FIX: Typing of FuncAnimation