Skip to content

frame_format to support all listed by animation writers #17909

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

Merged
merged 2 commits into from
Jul 15, 2020

Conversation

bmcfee
Copy link
Contributor

@bmcfee bmcfee commented Jul 13, 2020

PR Summary

Fixes #17908 . As documented in the issue, various animation writers support different formats, and there is no way to set the format directly from the API. This forces users to go through rcParams, but rcParams only listed a subset of frame formats.

This PR extends the supported frame formats in rcsetup.py to support any format supported by at least one writer. If the user selects a format that is not supported by their chosen writer, an exception will be raised at runtime (rather than at configuration time). This will allow a user to, for example, use svg format for jshtml output, which is not currently possible.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@@ -204,6 +204,15 @@ def test_Issue_1713(tmpdir):
assert rc.get('timezone') == 'UTC'


def test_animation_frame_formats():
# Animation frame_format should allow any of the following
# if any of these are not allowed, an exception will be raised
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally add another test checking the exception for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that, but I think that correctness is implied by an already existing test for validate_stringlist.

@QuLogic QuLogic merged commit 187e87b into matplotlib:master Jul 15, 2020
@QuLogic
Copy link
Member

QuLogic commented Jul 15, 2020

Thanks @bmcfee!

@QuLogic QuLogic added this to the v3.4.0 milestone Jul 15, 2020
@bmcfee
Copy link
Contributor Author

bmcfee commented Jul 15, 2020

Thanks for merging!

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

Successfully merging this pull request may close these issues.

rcParams restrictions on frame_formats are out of sync with supported values (HTMLWriter)
4 participants