Skip to content

rcParams restrictions on frame_formats are out of sync with supported values (HTMLWriter) #17908

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.

Sign up for GitHub

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

Jump to bottom
Closed
bmcfee opened this issue Jul 13, 2020 · 3 comments · Fixed by #17909
Closed

rcParams restrictions on frame_formats are out of sync with supported values (HTMLWriter) #17908

bmcfee opened this issue Jul 13, 2020 · 3 comments · Fixed by #17909
Labels
topic: animation
Milestone
v3.4.0

Comments

@bmcfee
Copy link
Contributor

bmcfee commented Jul 13, 2020

Bug report

jshtml animation uses HTMLWriter to render animation frames. Digging into the code for this, we see that HTMLWriter lists supported_formats:

matplotlib/lib/matplotlib/animation.py

Line 785 in c54a5a9

supported_formats = ['png', 'jpeg', 'tiff', 'svg']

However, there is no way to make use of this. The to_jshtml method of animation does not provide an interface to specify the frame format, so it must be pulled from rcParams. rcParams, in turn, does not allow svg as an option.

Code for reproduction

import matplotlib
matplotlib.rcParams['animation.frame_format'] = 'svg'

Actual outcome

# ... traceback cruft

ValueError: Key animation.frame_format: Unrecognized animation.frame_format string 'svg': valid strings are ['png', 'jpeg', 'tiff', 'raw', 'rgba']

Expected outcome

rcParams should allow setting frame formats that are supported by any animation writer, not just the ones supported by all.

Aside: rcParams allows raw but HTMLWriter doesn't. So there's already precedent.

Matplotlib version

  • Operating system: linux
  • Matplotlib version: 3.2.1
  • Matplotlib backend (print(matplotlib.get_backend())): Qt5Agg (but irrelevant to this issue)
  • Python version: 3.7.6
  • Jupyter version (if applicable): n/a
  • Other libraries: n/a

conda (main anaconda)
bmcfee added a commit to bmcfee/matplotlib that referenced this issue Jul 13, 2020
@bmcfee
fixes matplotlib#17908 , updates frame_format to support all listed b…
f2a5de4 
…y animation writers
@bmcfee bmcfee mentioned this issue Jul 13, 2020
Merged
6 tasks
@story645
Copy link
Member

Thanks for opening the PR against the rcparams. Would it be feasible/make sense to add it as a keyword argument to MovieWriter (and therefore it's subclasses)?

@bmcfee
Copy link
Contributor Author

bmcfee commented Jul 13, 2020 •
edited
Loading

Would it be feasible/make sense to add it as a keyword argument to MovieWriter (and therefore it's subclasses)?

I don't see why not. However, looking at the code for this, I think there might be a bigger family of changes to be made to expose writer behavior through the Animation object. For example, there's no obvious way to set loop/reflect behavior in to_jshtml, and the natural way to do it would be exposing keyword arguments like you suggest. (Nevermind, I was reading hastily. :))

@bmcfee
Copy link
Contributor Author

bmcfee commented Jul 14, 2020

To follow up on this, the current call to HTMLWriter only passes three arguments:

matplotlib/lib/matplotlib/animation.py

Lines 1359 to 1361 in 6b2cecc

writer = HTMLWriter(fps=fps,
embed_frames=embed_frames,
default_mode=default_mode)

but there are several more parameters that it could set:

matplotlib/lib/matplotlib/animation.py

Lines 796 to 798 in 6b2cecc

def __init__(self, fps=30, codec=None, bitrate=None, extra_args=None,
metadata=None, embed_frames=False, default_mode='loop',
embed_limit=None):

So we could thread through the frame_format parameter in this PR, but I think it might be better handled in a separate PR that focuses entirely on exposing internal animation parameters. This I think is independent of the issue in this thread, where the rcParams does not allow us to set a default at all currently. Fixing the rcparams check (#17909 ) seems necessary to me, exposing more API seems nice-to-have.

@QuLogic QuLogic added this to the v3.4.0 milestone Jul 15, 2020
@story645 story645 mentioned this issue Jul 15, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: animation
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging a pull request may close this issue.

frame_format to support all listed by animation writers bmcfee/matplotlib
4 participants
@QuLogic @bmcfee @story645 @dstansby