Support method_descriptor #46
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously serializing bare class methods would fail in Python 2
Now it works fine.
>>> cloudpickle.dumps(str.upper)
We accomplish this by extending pickle's reduce function registry.
In cases like ``dumps(pd.Series.sum)`` we used to depend on `obj.__self__`, which in this case is None. In these cases we should instead depend on `obj.__class__`
mrocklin
force-pushed
the
method-descriptor
branch
from
860da0a
to
93f31cd
Compare
|
With a test too! Thanks @mrocklin. I'm going to add you to the org for cloudpipe, though traffic is low. |
|
@mrocklin do you have other bugfixes like that in your pipe? If not I think those 2 fixes alone deserve a quick bugfix release + upload to PyPI. WDYT? |
| @@ -698,3 +701,18 @@ def _make_skel_func(code, closures, base_globals = None): | |||
| def _getobject(modname, attribute): | |||
| mod = __import__(modname, fromlist=[attribute]) | |||
| return mod.__dict__[attribute] | |||
|
|
|||
|
|
|||
| """ Use copy_reg to extend global pickle definitions """ | |||
There was a problem hiding this comment.
Style: better use # instead of inline string objects for commenting the code.
There was a problem hiding this comment.
Noted. Will do this in the future.
|
Regarding timeline, I hope to provide a fix for #45 a few weeks from now. I'm not sure how expensive a release is; I'll leave that judgement to you all |
|
Release is simple enough. I'm not reliant on this currently, it will be months before I touch cloudpipe again. |
|
+1 for making a quick bugfix release now then. I can give it a shot. |
|
Actually we had not done any release since 0.1.0. git log says that changes where mostly related to test improvements but as there are many of them and a couple of new features like #36 and support for class methods. So next release should be 0.2.0. We can wait a few weeks. |
rgbkrk
added a commit
to rgbkrk/spark
that referenced
this pull request
Jun 12, 2017
This brings in fixes and upgrades from the [cloudpickle](https://github.com/cloudpipe/cloudpickle) module, notably: * Import submodules accessed by pickled functions (cloudpipe/cloudpickle#80) * Support recursive functions inside closures (cloudpipe/cloudpickle#89, cloudpipe/cloudpickle#90) * Fix ResourceWarnings and DeprecationWarnings (cloudpipe/cloudpickle#88) * Assume modules with __file__ attribute are not dynamic (cloudpipe/cloudpickle#85) * Make cloudpickle Python 3.6 compatible (cloudpipe/cloudpickle#72) * Allow pickling of builtin methods (cloudpipe/cloudpickle#57) * Add ability to pickle dynamically created modules (cloudpipe/cloudpickle#52) * Support method descriptor (cloudpipe/cloudpickle#46) * No more pickling of closed files, was broken on Python 3 (cloudpipe/cloudpickle#32)
ghost
pushed a commit
to dbtsai/spark
that referenced
this pull request
Aug 22, 2017
## What changes were proposed in this pull request? Based on apache#18282 by rgbkrk this PR attempts to update to the current released cloudpickle and minimize the difference between Spark cloudpickle and "stock" cloud pickle with the goal of eventually using the stock cloud pickle. Some notable changes: * Import submodules accessed by pickled functions (cloudpipe/cloudpickle#80) * Support recursive functions inside closures (cloudpipe/cloudpickle#89, cloudpipe/cloudpickle#90) * Fix ResourceWarnings and DeprecationWarnings (cloudpipe/cloudpickle#88) * Assume modules with __file__ attribute are not dynamic (cloudpipe/cloudpickle#85) * Make cloudpickle Python 3.6 compatible (cloudpipe/cloudpickle#72) * Allow pickling of builtin methods (cloudpipe/cloudpickle#57) * Add ability to pickle dynamically created modules (cloudpipe/cloudpickle#52) * Support method descriptor (cloudpipe/cloudpickle#46) * No more pickling of closed files, was broken on Python 3 (cloudpipe/cloudpickle#32) * ** Remove non-standard __transient__check (cloudpipe/cloudpickle#110)** -- while we don't use this internally, and have no tests or documentation for its use, downstream code may use __transient__, although it has never been part of the API, if we merge this we should include a note about this in the release notes. * Support for pickling loggers (yay!) (cloudpipe/cloudpickle#96) * BUG: Fix crash when pickling dynamic class cycles. (cloudpipe/cloudpickle#102) ## How was this patch tested? Existing PySpark unit tests + the unit tests from the cloudpickle project on their own. Author: Holden Karau <holden@us.ibm.com> Author: Kyle Kelley <rgbkrk@gmail.com> Closes apache#18734 from holdenk/holden-rgbkrk-cloudpickle-upgrades.
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously serializing bare class methods would fail in Python 2
Now it works fine.
We accomplish this by extending pickle's reduce function registry.