-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Restore axes sharedness when unpickling. #10659
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
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.
LGTM. Needs a rebase
Previously, pickling and unpickling shared axes would result in axes sharing a ticker instance (because that's how shared axes are set up), but without changes of one's xlims propagated to the other. The reason is that that sharedness information is stored in AxesBase._shared_x_axes, which does *not* get pickled together with the Axes instance: the latter only has a textual reference "I am an instance of AxesBase", so the Grouper information is lost. To keep the Grouper information valid, instead move the Groupers to the instance dictionaries (as references to global groupers). Also make Groupers picklable following a similar strategy as Transforms, i.e. by transforming weakrefs into real refs when pickling and transforming them back into weakref when unpickling.
rebased |
I don't claim to fully understand what's going on here. However, all axes created in the session use the same global I'm not sure that this really creates a problem, but it's an asymmetry that I wouldn't have expected. |
The observation is correct, but not a problem... until we decide to allow post-hoc modifications of the sharedness state. See also #9923. |
Does that mean that the usual
isn't allowed? By whom? |
Well, I didn't know that API even existed (I think it qualifies as an API leak, but it's a nifty one...), so you have a point. |
I don't think it depends on whether the axes had been shared previously. In case of previously shared axes, you'd do
In case of previously unshared axes you can do the same:
|
My point is that with this implementation, you can't reload two separately pickled axes and then share them, and I think that's a reasonable tradeoff in exchange of being able to correctly pickle and unpickle previously shared axes. |
I'm afraid that someone will stumble upon this limitation again. |
So the I agree the detail that some Axes objects will use the global group and other axes (sets) will use their own private Grouper seems like a major gotcha bug. Suspect the fix will be to, in addition to this change, to prune the pickeled Grouper to just be things shared by current set of interest and then on restore to inject them selves back into the shared grouper. |
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.
A step in the right direction, but not convinced that the fix is worth the downside consequences.
Moved this to 3.1 |
Replaced by #11544. |
Previously, pickling and unpickling shared axes would result in
axes sharing a ticker instance (because that's how shared axes
are set up), but without changes of one's xlims propagated to the
other. The reason is that that sharedness information is stored in
AxesBase._shared_x_axes, which does not get pickled together with the
Axes instance: the latter only has a textual reference "I am an instance
of AxesBase", so the Grouper information is lost.
To keep the Grouper information valid, instead move the Groupers to the
instance dictionaries (as references to global groupers). Also make
Groupers picklable following a similar strategy as Transforms, i.e. by
transforming weakrefs into real refs when pickling and transforming them
back into weakref when unpickling.
PR Summary
PR Checklist