Skip to content

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

Closed
wants to merge 1 commit into from

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 3, 2018

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

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.0 milestone Mar 3, 2018
Copy link
Member

@jklymak jklymak left a 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.
@anntzer
Copy link
Contributor Author

anntzer commented May 2, 2018

rebased

@timhoffm
Copy link
Member

timhoffm commented May 5, 2018

I don't claim to fully understand what's going on here. However, all axes created in the session use the same global Grouper. In contrast, each set of axes that are created from one unpickling have a separate Grouper instance.

I'm not sure that this really creates a problem, but it's an asymmetry that I wouldn't have expected.

@anntzer
Copy link
Contributor Author

anntzer commented May 5, 2018

The observation is correct, but not a problem... until we decide to allow post-hoc modifications of the sharedness state.
Proper solution would be to implement merging of groupers, or just get rid of the grouper structure and simply store a list of axes that a given axes is shared with and walk the tree when needed (which is also O(n) unless you store way too many vertices).

See also #9923.

@ImportanceOfBeingErnest
Copy link
Member

until we decide to allow post-hoc modifications of the sharedness state

Does that mean that the usual

ax1.get_shared_x_axes().join(ax1, ax2)

isn't allowed? By whom?
In any case I think it would be useful not having the reestablish the sharing after unpickling. On the other hand, I'm not sure how often that really happens in the wild and looping though all axes and calling join isn't that hard either.

@anntzer
Copy link
Contributor Author

anntzer commented May 5, 2018

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.
In practice, though, that just means that the current implementation doesn't support post-hoc sharing of axises in manner in the way you suggested when using unpickled axes (because they end up using different groupers). But I think that's a relatively rarer case that just pickling and unpickling already shared axes...

@ImportanceOfBeingErnest
Copy link
Member

I don't think it depends on whether the axes had been shared previously.

In case of previously shared axes, you'd do

import matplotlib.pyplot as plt
import pickle

fig, axes = plt.subplots(ncols=3, sharey="row")
axes[0].plot([0,1],[0,1])
axes[1].plot([0,1],[1,2])
axes[2].plot([0,1],[2,3])

pickle.dump(fig, file('fig1.pkl', 'wb'))  

plt.close("all")

fig2 = pickle.load(file('fig1.pkl','rb'))
ax_master = fig2.axes[0]
for ax in fig2.axes:
    if ax is not ax_master:
        ax_master.get_shared_y_axes().join(ax_master, ax)
    
plt.show()

In case of previously unshared axes you can do the same:

import matplotlib.pyplot as plt
import pickle

fig, axes = plt.subplots(ncols=3)
axes[0].plot([0,1],[0,1])
axes[1].plot([0,1],[1,2])
axes[2].plot([0,1],[2,3])

pickle.dump(fig, file('fig1.pkl', 'wb'))  

plt.close("all")

fig2 = pickle.load(file('fig1.pkl','rb'))
ax_master = fig2.axes[0]
for ax in fig2.axes:
    if ax is not ax_master:
        ax_master.get_shared_y_axes().join(ax_master, ax)
    ax.autoscale()
    
plt.show()

@anntzer
Copy link
Contributor Author

anntzer commented May 6, 2018

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.

@timhoffm
Copy link
Member

timhoffm commented May 6, 2018

I'm afraid that someone will stumble upon this limitation again.
On the other hand, it feels like a proper solution with merging Groupers should be possible with just a few extra lines of code. I wouldn't hold this PR up from a release, but we should aim at full pickling support.

@tacaswell
Copy link
Member

So the Grouper object has a (weak)ref to every axes that is involved in any sharing does this mean that every pickle will include all currently shared axes, they will all be restored on unpickle, and then any non-used ones will be immediately gc'd (or do they stick around)?

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.

@tacaswell tacaswell modified the milestones: v3.0, v3.1 Jun 30, 2018
Copy link
Member

@tacaswell tacaswell left a 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.

@tacaswell
Copy link
Member

Moved this to 3.1

@anntzer
Copy link
Contributor Author

anntzer commented Jul 4, 2018

Replaced by #11544.

@anntzer anntzer closed this Jul 4, 2018
@anntzer anntzer deleted the pickleshared branch July 4, 2018 18:12
@QuLogic QuLogic removed this from the v3.1 milestone Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants