Skip to content

[DOC]: Add simple animation scatter plot to the example documentation #24096

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 5 commits into from
Oct 20, 2022

Conversation

kostyafarber
Copy link
Contributor

@kostyafarber kostyafarber commented Oct 5, 2022

PR Summary

This example comes off the back of #22374, the purpose being to provide a simple example of how to use PillowWriter to save an animation as a gif.

PR Checklist

Tests and Styling

  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@story645
Copy link
Member

story645 commented Oct 6, 2022

Looks good to me but @anntzer does the pillow stuff need to be commented out?

@jklymak
Copy link
Member

jklymak commented Oct 6, 2022

I actually find this a bit confusing. If the pillow stuff is commented out, why not just add the comments to an existing example

@story645
Copy link
Member

story645 commented Oct 6, 2022

I think seperately I'd really like a simple dot example since the rain scatter has kind of a lot going on in it.

@kostyafarber
Copy link
Contributor Author

@anntzer

@anntzer
Copy link
Contributor

anntzer commented Oct 7, 2022

Adding the instructions commented out to a simple preexisting example, e.g. https://matplotlib.org/devdocs/gallery/animation/simple_anim.html, seems fine to me, but an important point is that the title of the example should make it clear that saving as gif is showcased by that example (you don't want the user to click through all animation examples until finding out which one explains how to save as gif).

@kostyafarber
Copy link
Contributor Author

kostyafarber commented Oct 7, 2022

Adding the instructions commented out to a simple preexisting example, e.g. https://matplotlib.org/devdocs/gallery/animation/simple_anim.html, seems fine to me, but an important point is that the title of the example should make it clear that saving as gif is showcased by that example (you don't want the user to click through all animation examples until finding out which one explains how to save as gif).

Agreed. We can make the title more informative, if we go down the road of using this animation to illustrate saving as a gif. But before I make any changes, awaiting confirmation that we want to do this instead of just adding it to another example.

@story645
Copy link
Member

story645 commented Oct 7, 2022

My bias is that I think this example would be useful and a title like "Scatter Animation Saved as Gif" would keep all the relevant bits.

I'm not opposed to adding saving to an existing example instead, but one where a) adding to the title won't detract from the title b)the example isn't so complex that the the saving will get buried.

@kostyafarber
Copy link
Contributor Author

@anntzer @story645 any progress on what we want to do going ahead?

@anntzer
Copy link
Contributor

anntzer commented Oct 10, 2022

Sorry, I don't have the bandwidth to look more into this right now.

@story645
Copy link
Member

I'll approve if you change the title to "Animated Scatter Saved as Gif" but I don't know if anyone else will (though I don't think that's a blocker unless somebody explicitly blocks).

@kostyafarber
Copy link
Contributor Author

Done

@QuLogic
Copy link
Member

QuLogic commented Oct 13, 2022

We use sentence case, not title case, don't we? ➡ "Animated scatter saved as GIF" (but either way GIF should be capitalized as an acronym.)

@story645
Copy link
Member

🤦‍♀️yeah, sorry forgot! @kostyafarber please switch to sentence case.

@kostyafarber
Copy link
Contributor Author

@story645 done!

@story645
Copy link
Member

@kostyafarber let me know if I should merge and then a follow up is making this the saving all the things example or if you'll update this as a saving all the things example.

@kostyafarber
Copy link
Contributor Author

Hey let's just merge it and we can do a follow up PR. Thanks

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 20, 2022
@lumberbot-app
Copy link

lumberbot-app bot commented Oct 20, 2022

Something went wrong ... Please have a look at my logs.

It seems that the branch you are trying to backport to does not exist.

@QuLogic QuLogic modified the milestones: v3.6-doc, v3.6.2 Oct 20, 2022
QuLogic added a commit that referenced this pull request Oct 21, 2022
…096-on-v3.6.x

Backport PR #24096 on branch v3.6.x ([DOC]: Add simple animation scatter plot to the example documentation)
melissawm pushed a commit to melissawm/matplotlib that referenced this pull request Dec 19, 2022
…matplotlib#24096)

* add simple animation scatter plot

* change title to Animated Scatter Saved as Gif

* change title to sentence case
@ksunden ksunden mentioned this pull request Feb 20, 2023
6 tasks
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.

6 participants