-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix interaction with unpickled 3d plots. #16220
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
Conversation
7dfe6b8
to
1cb6c6d
Compare
I like the idea, but I have two concerns:
|
I actually think that's saner behavior (do you have actual examples of wanting to lose the callbacks when swapping canvases?).
I agree the API is not so nice, but _try_to_pickle would effectively become (semi)public API or at least need to be exposed both in connect() and mpl_connect(). I don't feel strongly about this, though. (And it should be do_pickle, not try_to_pickle, i.e. we should just error out when pickling fails.) |
Moved to 3.4 as this needs a bit more thought. |
See also https://discourse.matplotlib.org/t/mpl-iteractions-with-gui-outside-of-jupyter-notebooks/21523/6 for a case where attaching callbacks to the figure rather than the canvas makes more sense(?) |
On more consideration, I am convinced this is the right course. I suspect we want to make a much bigger deal of this, re-write the docs and start pushing people towards directly registering callbacks on the figures rather than silently changing this under the hood. |
That's fine with me, but do you want to take care of the docs rewrite? ;) |
In order to reset the mouse interaction callbacks on unpickled 3d plots, the callback registry really needs to be on the Figure object rather than the Canvas, because the canvas doesn't exist yet when the 3d axes is being unpickled (it is only set on the figure at the very end of unpickling). So move the callback registry to the figure (with a proxy property on the canvas). Then, add a private mechanism to pickle select callbacks, and combine everything together. Test with e.g. ``` import matplotlib.pyplot as plt import pickle fig = plt.figure() fig.add_subplot(111, projection='3d') p = pickle.dumps(fig) plt.close("all") pickle.loads(p) plt.show() ```
I think this (the callbacks pickling part) would be nice to have for #16931 (comment) as well. |
I do have a commit in this PR, but it is the deprecation note. |
PR Summary
In order to reset the mouse interaction callbacks on unpickled 3d plots,
the callback registry really needs to be on the Figure object rather
than the Canvas, because the canvas doesn't exist yet when the 3d axes
is being unpickled (it is only set on the figure at the very end of
unpickling).
So move the callback registry to the figure (with a proxy property on
the canvas).
Then, add a private mechanism to pickle select callbacks, and combine
everything together. (Bound methods of picklable objects are picklable.)
Test with e.g.
Closes #10843.
Goes on top of #15855. Would also be nice to get #16219 in first and rebase on top of it, but not compulsory.
The same mechanism would likely be usable e.g. for the "units finalize" callbacks (grep for this in axes/_base.py) which are likewise dropped on pickling/unpickling, but probably no one ever tried doing funky things with unit changes and pickling so no one noticed...
PR Checklist