-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Patch cloudpickle to not reset dynamic class each time it is unpickled #7388
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.
Thanks for the patch, a couple of minor things to resolve else looks good.
@@ -205,5 +208,99 @@ def test_numba_unpickle(self): | |||
self.assertIs(got1, got2) | |||
|
|||
|
|||
|
|||
class TestCloudPickleIssues(TestCase): |
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.
Maybe this ought to also include the original reproducer as a test? #7356 (comment)
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.
Good point. Test added in 8f6ffdc
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
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.
Thanks for the patch and fixes.
Patch cloudpickle to not reset dynamic class each time it is unpickled
Fixes a problem with cloudpickle when dynamic class is unpickled and the class is reset to the state when it is pickled.
A short reproducer for the cloudpickle problem is available here:
Note, cloudpickle treats any class created in
__main__
as dynamic.The patch makes cloudpickle remember whether a dynamic class is being reused. Reuse can come from the class being unpickled in the same process as it being pickled; or, a previously unpickled class is still around.
Reference an existing issue
Fixes #7356