Better MovieWriter init error message #13827
Merged
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
anntzer
reviewed
Apr 1, 2019
lib/matplotlib/animation.py
Outdated
| # extended with a mixin. This should be clearer in naming | ||
| # and description. For now, just give a reasonable error | ||
| # message to users. | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
Perhaps a TypeError, as that's what you get with ABC:
In [1]: class T(abc.ABC):
...: foo = abc.abstractmethod(lambda self: None)
...:
In [2]: T()
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-2-6d5ec5817329> in <module>
----> 1 T()
TypeError: Can't instantiate abstract class T with abstract methods foo
dstansby
previously requested changes
Apr 1, 2019
b72c2bf to
e93a434
Compare
e93a434 to
8c068ee
Compare
|
This is quite a mess. There were tests running on instances of the abstract MovieWriter. Fixed to the extent that you cannot instantiate MovieWriter at all, but need to subclass MovieWriter (whether reasonable or not). We do not want users to instantiate MovieWriter, but some tests seem to work with incomplete instances. Needs further sorting out, but beyond this PR. @dstansby Comment added to docstring. |
dstansby
approved these changes
Apr 2, 2019
tacaswell
approved these changes
Apr 2, 2019
meeseeksmachine
pushed a commit
to meeseeksmachine/matplotlib
that referenced
this pull request
Apr 2, 2019
dstansby
added a commit
that referenced
this pull request
Apr 2, 2019
…827-on-v3.1.x Backport PR #13827 on branch v3.1.x (Better MovieWriter init error message)
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.
PR Summary
From #5403 (comment). Calling
MovieWriter(fps=25)results in a not quite helpful error message'MovieWriter' object has no attribute 'args_key'.This is just a quick fix to give a reasonable error message, and get that improved user experience in with 3.1.
A full solution should add a description on the mixin mechanism, and maybe even join AbstractMovieWriter and MovieWriter classes. But that's beyond the scope of this PR.