Skip to content
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

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

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

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:

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)

@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

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:

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

but there are several more parameters that it could set:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants