FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Updates example and docstring to encourage the use of functools.partial in FuncAnimation by jeffreypaul15 · Pull Request #20358 · matplotlib/matplotlib · GitHub

Repository navigation

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

Closed
jeffreypaul15 wants to merge 5 commits into
matplotlib:mainfrom
jeffreypaul15:mpl-20326
Closed

jeffreypaul15 wants to merge 5 commits into
matplotlib:mainfrom
jeffreypaul15:mpl-20326

Conversation

jeffreypaul15 commented Jun 3, 2021 •
edited by oscargus
Loading

Copy link
Copy Markdown
Contributor

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.

Comment thread doc/api/animation_api.rst Outdated
@@ -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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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,

jeffreypaul15 Jun 3, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

So, would increasing the number of arguments be better?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread doc/api/animation_api.rst Outdated
return ln,

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This should address my issue too

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This makes more sense, thanks for the input

Comment thread lib/matplotlib/animation.py Outdated
jeffreypaul15 and others added 2 commits June 6, 2021 20:42
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Comment thread doc/api/animation_api.rst Outdated
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
jklymak added this to the v3.5.0 milestone Jun 10, 2021
Comment thread doc/api/animation_api.rst
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation
from functools import partial

jklymak Jun 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
from functools import partial

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Aren't we using partial here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not that I can see...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sorry, I got confused with the example below

Comment thread doc/api/animation_api.rst
ax.set_ylim(-1, 1)
return ln,

def update(frame, x, y):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I thought the same, hence the use of offset, should I revert to that instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Jul 7, 2021

Copy link
Copy Markdown
Member

@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 commented Nov 2, 2022

Copy link
Copy Markdown
Member

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL