ENH: Create an abstract base class for movie writers. #5454
Conversation
|
If there is interest in this, it will need a unit test or two. I'm looking into that now. |
|
Updated with unit tests. |
|
I changed the PR title to "WIP". The minimum requirements for making a movie writer class registerable should at least be better documented, if not actually part of the abstract base class. Or there could be a subclass, say, |
|
For the record, here's what the abstract base classes look like if the class class AbstractMovieWriter(six.with_metaclass(abc.ABCMeta)):
"""
Instances of a concrete subclass of this class may be used as the
`writer` argument of Animation.save()
"""
@abc.abstractmethod
def setup(self, fig, outfile, dpi, *args):
pass
@abc.abstractmethod
def grab_frame(self, **savefig_kwargs):
pass
@abc.abstractmethod
def finish(self):
pass
@contextlib.contextmanager
def saving(self, *args):
self.setup(*args)
yield
self.finish()
class AbstractRegisteredMovieWriter(AbstractMovieWriter):
"""
A concrete subclass of this class may be registered as a writer,
e.g.
from matplotlib.animation import writers, AbstractRegisteredMovieWriter
@writers.register("foo")
class FooMovieWriter(AbstractRegisteredMovieWriter):
...
The signature of `__init__` is required to match how an instance of
the class is created within the function `Animation.save()` when a string
(i.e. a registered name) is passed as the `writer` argument.
The `isAvailable` class method is required by the `register` method of the
`MovieWriterRegistry` class.
"""
@abc.abstractmethod
def __init__(self, fps=5, codec=None, bitrate=None, extra_args=None,
metadata=None):
pass
@abc.abstractclassmethod # Python 3 only! There are workarounds for Python 2.
def isAvailable(cls):
passI used |
|
attn @dopplershift @WeatherGod |
|
I am 👍 on merging this, but will defer to @dopplershift and @WeatherGod on final merging. |
|
👍 from me as well. |
|
Looks good to me. Probably needs a whats_new entry. |
Would this end up in 1.5.1, or 1.6? |
|
Can you please put it in its own file (see the readme). This will be for 2.1 On Sun, Nov 29, 2015, 21:27 Warren Weckesser notifications@github.com
|
Done. One of the tests failed, but the failure doesn't appear to be related to this PR. |
|
All the tests are now passed, so to whoever or whatever reran them: thanks! |
|
You are right they were random failures. I pressed the rerun button in the Travis interface |
|
@jenshnielsen Thanks. I'll fix the new merge conflicts when I get a chance. |
|
Rebased, and the tests still pass. |
|
This is looking real good. Just one question since I have never created abc's before. Does it make sense to have the |
|
I couldn't think of any reason to not include it. In general, there's nothing wrong with abcs having concrete methods. In this case, I guess the alternative would be to make it an abstract method, and require that a concrete subclass implement it, but I suspect 99% of all implementations will be exactly what is now in the abc, so we might as well include the implementation in the abc, and reduce the amount of boilerplate code required in a subclass. |
| This class is set up to provide for writing movie frame data to a pipe. | ||
| saving() is provided as a context manager to facilitate this process as:: | ||
|
|
||
| with moviewriter.saving('myfile.mp4'): |
There was a problem hiding this comment.
should this be
with moviewriter.saving(fig, 'myfile.mp4', 100):There was a problem hiding this comment.
Ah, right. The docstrings are only slightly edited versions of the original docstrings from the MovieWriter class. Copy-edit suggestions are welcome.
After looking at this again, I now think the signature of the saving method in the abc should be def saving(self, fig, outfile, dpi, *args). This API is based on how a writer is used in the saving method of the Animation class. There it is only called as with writer.saving(self._fig, filename, dpi):. Even *args could be dropped from the signature.
|
Don't merge yet. I'm going to tweak the API a bit--see my earlier response to @tacaswell's inline comment. |
|
@WarrenWeckesser Any update on this? |
|
Not yet. Other projects and the holidays pushed this down the to-do list. I'll get back to it in the next day or so. |
|
No worries! On Sat, Dec 26, 2015 at 1:52 PM Warren Weckesser notifications@github.com
|
Add the abstract base class AbstractMovieWriter to animation.py. This is now the parent of MovieWriter. AbstractMovieWriter defines the interface required by objects that are to be given as the 'writer' argument of Animation.save(). This will help third-party developers create classes that can be used to create movies.
|
Updated, rebased, and ready to go. |
|
Looks good. Thanks |
ENH: Create an abstract base class for movie writers.
Add the abstract base class AbstractMovieWriter to animation.py.
This is now the parent of MovieWriter. AbstractMovieWriter
defines the interface required by objects that are to be given
as the 'writer' argument of Animation.save().
This will help third-party developers create classes that can
be used to create movies.