-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Doc animation #7589
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.
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
Doc animation #7589
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this--these are a huge improvement to what I wrote originally. The initial preamble with the example I think should help a lot of people.
One question: Should subclasses be duplicating the documentation of attributes from parent classes?
Otherwise, just a bunch of little stuff.
FuncAnimation | ||
ArtistAnimation | ||
|
||
In both cases it is critical to keep a reference to tho instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tho
-> the
ax.set_ylim(-1, 1) | ||
return ln, | ||
|
||
def update(i): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to replace i
with something making it clear it's a value pulled from the frames
kwarg?
|
||
The second method is to us `functools.partial` to 'bind' artists to | ||
function. A third method is to use closures to build up the required | ||
artists and functions. A forth method is to create a class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forth
-> fourth
@@ -68,6 +68,8 @@ def adjusted_figsize(w, h, dpi, n): | |||
|
|||
# A registry for available MovieWriter classes | |||
class MovieWriterRegistry(object): | |||
'''Registry of of available writer classes by human readable name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double of
@@ -109,6 +111,15 @@ def list(self): | |||
return list(self.avail.keys()) | |||
|
|||
def is_available(self, name): | |||
'''If given writer is available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about '''Check if given writer is available.'''
?
|
||
def func(fr: object, *fargs) -> iterable_of_artists: | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not Sure if this is an extra blank line or one needed to surround the above code block...
If `None`, then equivalent to passing ``itertools.count`` | ||
|
||
init_func : callable, optional | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line
The number of values from `frames` to cache. | ||
|
||
interval : number, optional | ||
Delay between frames. Defaults to 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Period after 200?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Units?
used. This function will be called once before the first frame. | ||
repeat: bool, optional | ||
controls whether the animation should repeat when the sequence | ||
of frames is completed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add "Defaults to True
."
*frames* can be a generator, an iterable, or a number of frames. | ||
repeat_delay : number, optional | ||
If the animation in repeated, adds a delay in milliseconds | ||
before repeating the animation. Defaults to None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"None" -> "None
."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typos and such.
hence the timers), will be garbage collected which will stop the | ||
animation. | ||
|
||
To save an animation use to disk use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove first 'use'.
Animation | ||
========= | ||
|
||
The easiest way to make a live animation in mpl is to use one of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mpl -> Matplotlib
The inner workings of `FuncAnimation` is more-or-less:: | ||
|
||
for d in frames: | ||
arts = func(d, *fargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason why this can't say artists
instead of arts
.
|
||
'Blitting' is a `old technique | ||
<https://en.wikipedia.org/wiki/Bit_blit>`__ in computer graphics. The | ||
general gist is to take as existing bit map (in our case a mostly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as -> an
|
||
ax = fig.gca() | ||
|
||
def update_blit(arts): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, artists
for this whole block.
|
||
If a generator function, then must have the signature :: | ||
|
||
def gen_function(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be -> int
or -> float
or anything?
The number of values from `frames` to cache. | ||
|
||
interval : number, optional | ||
Delay between frames. Defaults to 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Units?
*init_func* is a function used to draw a clear frame. If not given, the | ||
results of drawing from the first item in the frames sequence will be | ||
used. This function will be called once before the first frame. | ||
repeat: bool, optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space before colon.
other needed events. | ||
|
||
interval : number, optional | ||
Delay between frames. Defaults to 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Units?
be disabled for other frames. | ||
|
||
interval : number, optional | ||
Delay between frames. Defaults to 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Units?
I think I have addressed everything and re-worked the section on writers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more minor things. Otherwise, looks good.
AVConvFileWriter | ||
|
||
|
||
Fundamentally, a `MovieWriter` does is provide is a way to grab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'what' got removed, so now the sentence is a bit off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the second 'is' is still there.
@@ -463,6 +483,12 @@ def cleanup(self): | |||
# Base class of ffmpeg information. Has the config keys and the common set | |||
# of arguments that controls the *output* side of things. | |||
class FFMpegBase(object): | |||
'''Mixin class for FFMpeg output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit pedantic, but the first line on each of these newly documented classes sometimes does and sometimes doesn't end with a period.
be disabled for other frames. | ||
|
||
interval : number, optional | ||
Delay between frames in miliseconds. Defaults to 200. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
miliseconds -> milliseconds
of frames is completed. | ||
repeat_delay : number, optional | ||
If the animation in repeated, adds a delay in milliseconds | ||
before repeating the animation. Defaults to None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one's missing the backticks on None
.
used. This function will be called once before the first frame. | ||
repeat : bool, optional | ||
Controls whether the animation should repeat when the sequence | ||
of frames is completed. Defaults to `True` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one's missing the period.
sequential frames from the same underlying `~matplotlib.figure.Figure` | ||
object. The base class `MovieWriter` implements 3 methods and a | ||
context manager. The only difference between the pipe-based and | ||
file-based writers in the arguments to their respective ``setup`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in ->is in
file to disk. For example :: | ||
|
||
moviewriter = MovieWriter(...) | ||
moveiewriter.setup(fig=fig, 'my_movie.ext', dpi=100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moveiewriter -> moviewriter
default to ``rcParams['animation.codec']`` | ||
|
||
bitrate : number, optional | ||
Specifies the amount of bits used per second in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amount -> number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few truly pedantic things that I would feel back actually requiring before merging...
@@ -1199,7 +1199,7 @@ class TimedAnimation(Animation): | |||
|
|||
repeat_delay : number, optional | |||
If the animation in repeated, adds a delay in milliseconds | |||
before repeating the animation. Defaults to None | |||
before repeating the animation. Defaults to `None` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pedantic, but no period at the end.
|
||
''' | ||
|
||
def __init__(self, fps=5, codec=None, bitrate=None, extra_args=None, | ||
metadata=None): | ||
''' | ||
'''MovieWriter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No period at the end--though not really a sentence, either. 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally decided against a .
here (due to it not being a sentence).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough.
@@ -1304,24 +1279,24 @@ class ArtistAnimation(TimedAnimation): | |||
other needed events. | |||
|
|||
artists : list | |||
with each list entry a collection of artists that | |||
With each list entry a collection of artists that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead just: Each list entry is a collection...
The plan (if I have time to get to it is to flesh out the FuncAnimation section with the same example implemented the other 3 ways, but I think that as-is it is still a major improvement.