Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Updates example and docstring to encourage the use of functools.partial in FuncAnimation#20358
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I thinkbind 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,
jeffreypaul15Jun 3, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
doc/api/animation_api.rst Outdated
| return ln, | ||
| ani = FuncAnimation( | ||
| fig, partial(update, offset=-0.5), |
There was a problem hiding this comment.
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 whatpartial is supposed to do. After this I am still mystified 😉. Why wouldn't I just passupdate directly here? I don't see whereoffset ever gets used, so I don't see what the -0.5 means.
There was a problem hiding this comment.
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 updateupdate(), I'll make this clearer.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 (asupdate is closing overxdata andydata). Maybe keep this example dropping the partial to show the closure and switch this example to something like:
defupdate(frame,x,y):x.append(...)y.append(....) ...xdata= []ydata= []ani=FuncAnimation(fig,partial(update,x=xdata,y=ydata), ...)
There was a problem hiding this comment.
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
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
| @@ -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 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| from functools import partial |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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 ofoffset, should I revert to that instead?
There was a problem hiding this comment.
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 ;-)
There was a problem hiding this comment.
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 :
importnumpyasnpimportmatplotlib.pyplotaspltfrommatplotlib.animationimportFuncAnimationfromfunctoolsimportpartialfig,ax=plt.subplots()ln,=plt.plot([], [],'ro')definit():ax.set_xlim(0,2*np.pi)ax.set_ylim(-1,1)returnln,defupdate(frame,x_offset=0,y_offset=0):xdata.append(frame+x_offset)ydata.append(np.sin(frame)+y_offset)ln.set_data(xdata,ydata)returnln,x_offset=2y_offset=0.2ani=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()
There was a problem hiding this comment.
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 practicallybetter than just using a global, we should write such an example.
@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! |
@jeffreypaul15 Thank you for this! It ended up in#24238 with some other modifications. |
Uh oh!
There was an error while loading.Please reload this page.
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.