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

add animations to plot_ppc #546

Merged
merged 3 commits into from
Jan 23, 2019
Merged

add animations to plot_ppc #546

merged 3 commits into from
Jan 23, 2019

Conversation

aloctavodia
Copy link
Contributor

Some examples

kind=density
00_animated

kind=scatter
01_animated

kind=cumulative
02_animated

kind=density discrete variables
03_animated

@ahartikainen
Copy link
Contributor

Is there a way to increase number of samples from 1 to N? e.g. 3

animate,
np.arange(0, num_pp_samples),
init_func=init,
blit=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will break stuff (at least it did on osx) and is probably not needed if animation is saved to a file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not clear to me what will break stuff. The idea is to be able to work with animations inside a Jupyter notebook, not just save them to a file.

Copy link
Contributor

Choose a reason for hiding this comment

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

blit=True

Not sure what is the situation now

matplotlib/matplotlib#531

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, what we can do here, add a keyword so users can set the value of blit?, or we detect the OS and if it is osx we use TkAgg? (BTW I don't have access to a machine with osx)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think let users decide. Maybe throw warning on osx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you like something like "If you experience problems rendering the animation try turning blit to False or setting the plotting backend to TkAgg".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or better "If you experience problems rendering the animation try setting `animation_kwargs({'blit':False}) or setting the plotting backend to TkAgg."

Copy link
Contributor

Choose a reason for hiding this comment

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

Latter

@aloctavodia
Copy link
Contributor Author

Do you mean to render N samples per frame?

@ahartikainen
Copy link
Contributor

Yes. Like having 3 to 10 samples per frame.

@aloctavodia
Copy link
Contributor Author

Not with the current implementation but it can be added. A friend of mine suggested to have an option to pile-up samples, probably with a lower value of alpha. My concern with these options is if they goes against the purpose of the animation, that is to minimize "deterministic construal errors", but honestly I have to learn more about this and about Hypothetical Outcome Plots.

@aloctavodia
Copy link
Contributor Author

@ahartikainen If you don't mind I prefer to left the N-samples per frame for another PR, after I have the time to do some more reading. I could open an issue.

@aloctavodia
Copy link
Contributor Author

@ahartikainen do you think this is ready to merge? (and further improve in the future)

@ahartikainen
Copy link
Contributor

Yes, LGTM, sorry I have missed your question before.

@ahartikainen ahartikainen merged commit 86b37d0 into master Jan 23, 2019
@ahartikainen ahartikainen deleted the animated_ppc branch January 27, 2019 08:16
@ColCarroll ColCarroll mentioned this pull request Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants