Connect the Animation event source callback in the constructor. #26774
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
greglucas
approved these changes
Nov 20, 2024
|
@dopplershift Perhaps you can comment on this change as well? |
dopplershift
approved these changes
Mar 10, 2025
|
Sound also reasonable to me. Why can't we merge? I looks like the "Prevent Merging" action got hung up. @anntzer please rebase. |
Currently, there is no "official" way of knowing whether a Figure hosts an Animation. Yet, this information can be of interest e.g. for third-party backends. For example, a variant of the ipython inline backend could choose to actually animate its (inline) output in that case; indeed, this feature was already supported by the itermplot backend. itermplot is actually now outdated but the general idea can still be made to work -- I have a working patch for mplterm. The idea is for the backend to override FigureCanvas.new_timer (on the backend-specific canvas subclass) to internally register any timer that gets created (e.g. by the animation module), then, when the canvas is show()n, to first trigger a fake draw_event so that animation callbacks (if any) get attached to the timer, and then to introspect what callbacks have been attached to the timer and retrieve any Animation instance from that. While this all works, there is brittleness on a specific point, namely the need to trigger the fake draw_event (as the callback only gets attached in Animation._start) early in show(): at that point there may be no renderer available so one cannot construct a real DrawEvent (in fact, itermplot just sets renderer to None). If there are additional (end-user-provided) draw_event handlers connected, they may well error out on this fake draw_event. Yet, it is easy to work around this problem, by connecting the animation stepping callback immediately to the timer when the Animation is constructed, rather than waiting for the first draw. The timer is still only *started* at the first draw, so nothing changes from the point of view of the end user. Note that this does not create a strong reference holding the Animation in the "usual" case (of backends that don't keep a reference to the timers they create) -- it's just a reference loop of the Animation holding the Timer holding an Animation method as a callback.
|
rebased |
timhoffm
approved these changes
Mar 11, 2025
The PR was just older than the action, so it was never run for it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, there is no "official" way of knowing whether a Figure hosts an Animation. Yet, this information can be of interest e.g. for third-party backends. For example, a variant of the ipython inline backend could choose to actually animate its (inline) output in that case; indeed, this feature was already supported by the itermplot backend. itermplot is actually now outdated but the general idea can still be made to work -- I have a working patch for mplterm.
The idea is for the backend to override FigureCanvas.new_timer (on the backend-specific canvas subclass) to internally register any timer that gets created (e.g. by the animation module), then, when the canvas is show()n, to first trigger a fake draw_event so that animation callbacks (if any) get attached to the timer, and then to introspect what callbacks have been attached to the timer and retrieve any Animation instance from that.
While this all works, there is brittleness on a specific point, namely the need to trigger the fake draw_event (as the callback only gets attached in Animation._start) early in show(): at that point there may be no renderer available so one cannot construct a real DrawEvent (in fact, itermplot just sets renderer to None). If there are additional (end-user-provided) draw_event handlers connected, they may well error out on this fake draw_event.
Yet, it is easy to work around this problem, by connecting the animation stepping callback immediately to the timer when the Animation is constructed, rather than waiting for the first draw. The timer is still only started at the first draw, so nothing changes from the point of view of the end user.
Note that this does not create a strong reference holding the Animation in the "usual" case (of backends that don't keep a reference to the timers they create) -- it's just a reference loop of the Animation holding the Timer holding an Animation method as a callback. Furthermore, the callback could easily be replaced by a WeakMethod to completely get rid of the reference loop.
(If we agree on the change here I can also add a test that explicitly checks for this early-binding.)
Edit: Actually, a much simpler and more general issue with the "trigger draw to see what gets connected to the timer" is that that also starts the timer and therefore subsequent introspection becomes racy and may be too slow to catch the first frames.
PR summary
PR checklist