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

Updates example and docstring to encourage the use of functools.partial in FuncAnimation #20358

Closed
wants to merge 5 commits into from

Conversation

jeffreypaul15
Copy link
Contributor

@jeffreypaul15 jeffreypaul15 commented Jun 3, 2021

Closes #20326
Example for FuncAnimation is updated with the use of functools.partial.
A note is added in the docstring as well to use partial instead of fargs.

@@ -124,7 +125,36 @@ artist at a global scope and let Python sort things out. For example ::
plt.show()

The second method is to use `functools.partial` to 'bind' artists to
Copy link
Member

Choose a reason for hiding this comment

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

I think bind is doing too much work here. What about partial is used here to pass arguments to the update method - which separately is kind of confusing since this update method only takes one argument,

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, would increasing the number of arguments be better?

Copy link
Member

Choose a reason for hiding this comment

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

I think it could make it clearer what partial is doing here

return ln,

ani = FuncAnimation(
fig, partial(update, offset=-0.5),
Copy link
Member

Choose a reason for hiding this comment

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

I guess I'm a good guinea pig for this, as I don't really know what partial is supposed to do. After this I am still mystified 😉. Why wouldn't I just pass update directly here? I don't see where offset ever gets used, so I don't see what the -0.5 means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, my bad. I forgot to update update(), I'll make this clearer.

Copy link
Member

Choose a reason for hiding this comment

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

This should address my issue too

Copy link
Member

Choose a reason for hiding this comment

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

This example is also using closures (as update is closing over xdata and ydata). Maybe keep this example dropping the partial to show the closure and switch this example to something like:

def update(frame, x, y):
    x.append(...)
    y.append(....)
    ...

xdata = []
ydata = []
ani = FuncAnimation(fig, partial(update, x=xdata, y=ydata), ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes more sense, thanks for the input

lib/matplotlib/animation.py Outdated Show resolved Hide resolved
jeffreypaul15 and others added 2 commits June 6, 2021 20:42
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
doc/api/animation_api.rst Outdated Show resolved Hide resolved
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@jklymak jklymak added this to the v3.5.0 milestone Jun 10, 2021
@@ -103,6 +103,7 @@ artist at a global scope and let Python sort things out. For example ::
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation
from functools import partial
Copy link
Member

@jklymak jklymak Jun 10, 2021

Choose a reason for hiding this comment

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

Suggested change
from functools import partial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't we using partial here?

Copy link
Member

Choose a reason for hiding this comment

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

Not that I can see...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I got confused with the example below

ax.set_ylim(-1, 1)
return ln,

def update(frame, x, y):
Copy link
Member

Choose a reason for hiding this comment

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

This is just wrong isn't it? you are appending to x and y and then setting the data with xdata and ydata. I guess I am never clear if x and y are references or not, but at the very least this is confusing...

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 thought the same, hence the use of offset, should I revert to that instead?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know. I'd not do it this way, so whoever would do it this way should write the doc ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tacaswell Suggested the example, would this be better instead :

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation
from functools import partial

fig, ax = plt.subplots()
ln, = plt.plot([], [], 'ro')

def init():
   ax.set_xlim(0, 2*np.pi)
   ax.set_ylim(-1, 1)
   return ln,

def update(frame, x_offset=0, y_offset=0):
   xdata.append(frame+x_offset)
   ydata.append(np.sin(frame)+y_offset)
   ln.set_data(xdata, ydata)
   return ln,

x_offset = 2
y_offset = 0.2
ani = FuncAnimation(
fig, partial(update, x_offset=x_offset, y_offset=y_offset),
frames=np.linspace(0, 2 * np.pi, 128),
init_func=init, blit=True)

plt.show()

Copy link
Member

Choose a reason for hiding this comment

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

I mean the above looks better to me. But I don't understand, at all, why we wouldn't just use globals here. If an example where this is practically better than just using a global, we should write such an example.

@jklymak
Copy link
Member

jklymak commented Jul 7, 2021

@jeffreypaul15 what is the status of this. I've moved to draft, but feel free to move back if you think it is reviewable, or close if you don't pan to move forward. Thanks!

@oscargus
Copy link
Contributor

oscargus commented Nov 2, 2022

@jeffreypaul15 Thank you for this! It ended up in #24238 with some other modifications.

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.

FuncAnimation Named Arguments
6 participants