Skip to content
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

Fix for unpickling polar plot issue #4068 #4264

Merged
merged 4 commits into from Mar 25, 2015
Merged

Fix for unpickling polar plot issue #4068 #4264

merged 4 commits into from Mar 25, 2015

Conversation

olanmatt
Copy link

This implements a fix for the bug documented in issue #4068

PolarAffine of lib/matplotlib/projections/polar.py returned an empty state when being pickled because of an unimplemented __getstate__ function. Removing this function resolved the reported exception and restored the expected behaviour.

@tacaswell tacaswell added this to the next point release milestone Mar 23, 2015
@jenshnielsen
Copy link
Member

Travis failure in docs build is not related to this Pull request. Fixed by #4267

@ghost
Copy link

ghost commented Mar 23, 2015

@jenshnielsen Thanks for the heads up! Would it be possible to get the tests restarted?

@tacaswell
Copy link
Member

cc @pelson

It looks like Phil added this code in 7885d94 in PR #1175

@ghost
Copy link

ghost commented Mar 23, 2015

cc @pelson @tacaswell

Here's what we've noticed.

The problem arises because PolarAffine is overriding the __getstate__() function of Transform, but all it does is return an empty dictionary currently. As such, when Pickle is used to restore the instance of a PolarAffine, all of its attributes
are lost and not set. Therefore, the AttributeError exception was being raised.

It seems like other children of Transform seem to just use their parent's __getstate__ and __setstate__ functions - which seem to work well with pickle.

@tacaswell
Copy link
Member

Can you add a test? https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_pickle.py has some tests already (and while you are in there can you add an @cleanup to all of the tests?).

def __getstate__(self):
return {}


Copy link
Member

Choose a reason for hiding this comment

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

Nit-pick: We leave 2 newlines between top level module objects. (I think the PEP8 tests will probably catch this)

@pelson
Copy link
Member

pelson commented Mar 24, 2015

I'm OK with the change. We can't be certain that this wont break other classes, which may have attributes which aren't picklable, but we can deal with that as the occur (should be a simple fix when it crops up).

👍

@dkua
Copy link
Contributor

dkua commented Mar 25, 2015

Whitespace issue fixed, @cleanup decorator added to tests, and a minimum test created.

There was a funny error in one of the Travis test suites, does it simply need a restart?

@tacaswell
Copy link
Member

Yes, I restarted that test. Looks like apt timed out or some-such.

@tacaswell
Copy link
Member

Thank you!

tacaswell added a commit that referenced this pull request Mar 25, 2015
BUG : Fix for unpickling polar plot

closes #4068
@tacaswell tacaswell merged commit 174fc94 into matplotlib:master Mar 25, 2015
tacaswell added a commit that referenced this pull request Mar 25, 2015
BUG : Fix for unpickling polar plot

closes #4068
@tacaswell
Copy link
Member

cherry-picked to color_overhaul as 592818a

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.

None yet

5 participants