ENH: Create an abstract base class for movie writers.

[WarrenWeckesser]

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.

[WarrenWeckesser]

If there is interest in this, it will need a unit test or two. I'm looking into that now.

[WarrenWeckesser]

Updated with unit tests.

[WarrenWeckesser]

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, AbstractRegisterableMovieWriter(AbstractMovieWriter), that includes the __init__ and isAvailable methods as abstract methods (but the current required signature of __init__ includes arguments that not all movie writers need). Anyway, this change needs some more thought.

[WarrenWeckesser]

For the record, here's what the abstract base classes look like if the class AbstractRegisteredMovieWriter is implemented (complete docstrings not included).

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):
        pass

I used abc.abstractclassmethod in the definition of AbstractRegisteredMovieWriter, but that is not available in Python 2. Work-arounds are available, but since this is just for discussion at the moment, I won't clutter up the code with addtional Python 2/3 compatibility code.

[tacaswell]

attn @dopplershift @WeatherGod

[tacaswell]

I am 👍 on merging this, but will defer to @dopplershift and @WeatherGod on final merging.

[dopplershift]

👍 from me as well.

[WeatherGod]

Looks good to me. Probably needs a whats_new entry.

[WarrenWeckesser]

Probably needs a whats_new entry.

Would this end up in 1.5.1, or 1.6?

[tacaswell]

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
wrote:

Probably needs a whats_new entry.

Would this end up in 1.5.1, or 1.6?

—
Reply to this email directly or view it on GitHub
#5454 (comment)
.

[WarrenWeckesser]

Probably needs a whats_new entry.

Done.

One of the tests failed, but the failure doesn't appear to be related to this PR.

[WarrenWeckesser]

All the tests are now passed, so to whoever or whatever reran them: thanks!

[jenshnielsen]

You are right they were random failures. I pressed the rerun button in the Travis interface

[WarrenWeckesser]

@jenshnielsen Thanks.

I'll fix the new merge conflicts when I get a chance.

[WarrenWeckesser]

Rebased, and the tests still pass.

[WeatherGod]

This is looking real good. Just one question since I have never created abc's before. Does it make sense to have the saving() method defined in the abstract base class?

[WarrenWeckesser]

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.

[tacaswell]

should this be

with moviewriter.saving(fig, 'myfile.mp4', 100):

[WarrenWeckesser]

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.

[WarrenWeckesser]

Don't merge yet. I'm going to tweak the API a bit--see my earlier response to @tacaswell's inline comment.

[tacaswell]

@WarrenWeckesser Any update on this?

[WarrenWeckesser]

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.

[tacaswell]

No worries!

On Sat, Dec 26, 2015 at 1:52 PM Warren Weckesser notifications@github.com
wrote:

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.

—
Reply to this email directly or view it on GitHub
#5454 (comment)
.

[WarrenWeckesser]

Updated, rebased, and ready to go.

[jenshnielsen]

Looks good. Thanks