BUG: Fix bug pickling namedtuple. #113

ssanderson · 2017-08-05T19:10:03Z

Fixes a crash when trying to set __dict__ onto a type when __dict__
is a property.

ssanderson · 2017-08-05T19:10:41Z

cloudpickle/cloudpickle.py

-            # don't extract dict that are properties
-            clsdict.pop('__dict__', None)
-            clsdict.pop('__weakref__', None)
+        clsdict.pop('__weakref__', None)


This now unconditionally pops __weakref__, since it's not meaningful to pickle it.

ssanderson · 2017-08-05T19:12:18Z

cloudpickle/cloudpickle.py


-        # hack as __new__ is stored differently in the __dict__


I'm not sure what this was trying to do previously, but it causes unpickling of namedtuples to not work, and the test suite still passes for me without it.

rgbkrk · 2017-08-07T18:08:58Z

Want to rebase this?

I'd love for us to make a release soon.

rgbkrk · 2017-08-09T21:11:14Z

I'm going to go ahead and make a release by the end of the week.

ssanderson · 2017-08-09T21:16:31Z

Sounds good. I don't think I'm going to have time to look at this before then (the current patch was failing on a couple python versions before the merge conflict).

…

On Wed, Aug 9, 2017 at 5:11 PM, Kyle Kelley ***@***.***> wrote: I'm going to go ahead and make a release by the end of the week. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABg8hbDBbQzgUqW9Wu-5HtwoXW1JsDMzks5sWiBzgaJpZM4Ouih4> .

rgbkrk · 2017-08-09T22:45:28Z

Ok, I'll go ahead and release then.

robertnishihara · 2017-08-15T18:24:19Z

tests/cloudpickle_test.py

@@ -652,6 +652,15 @@ def __init__(self, x):

        self.assertEqual(set(weakset), set([depickled1, depickled2]))

+    def test_namedtuple(self):


What do you think about also testing that we can pickle/unpickle MyTuple (the class itself).

codecov-io · 2017-08-30T23:01:40Z

Codecov Report

Merging #113 into master will increase coverage by 0.83%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #113      +/-   ##
==========================================
+ Coverage   84.07%   84.91%   +0.83%     
==========================================
  Files           2        2              
  Lines         559      570      +11     
  Branches      107      107              
==========================================
+ Hits          470      484      +14     
+ Misses         63       61       -2     
+ Partials       26       25       -1

Impacted Files	Coverage Δ
cloudpickle/cloudpickle.py	`84.83% <100%> (+0.83%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16cacb0...6fb05ca. Read the comment docs.

Fixes a crash when trying to set `__dict__` onto a type when `__dict__` is a property.

rgbkrk · 2017-10-22T19:36:50Z

Looks like we need a special case for Python 2.6 and pypy. Honestly though, I'm personally ok with dropping 2.6 support. What do you think @mrocklin, @ogrisel, and @holdenk?

mrocklin · 2017-10-22T19:40:13Z

I'm comfortable with dropping 2.6 generally.

ssanderson · 2017-10-22T19:58:36Z

I can take a look this evening to see if there's a straightforward way to fix this on pypy.

On PyPy, built-in type constructors aren't necessarily builtin_function_or_method instances, so we need to check for them in `save_function` rather than just in `save_builtin_function`.

ssanderson · 2017-10-22T21:19:38Z

The issue on PyPy was that the builtin type constructors are regular functions rather than instances of builtin_function_or_method, so the special-case for them wasn't getting hit on pypy. I moved that special case branch and it appears to have fixed the 2.6 build as well.

ssanderson · 2017-10-25T16:44:57Z

I think this is good to merge unless anyone has comments/concerns (@rgbkrk maybe?).

rgbkrk · 2017-10-25T18:52:52Z

This is great, we can ship as well I think.

rgbkrk · 2017-10-25T18:55:02Z

This is probably fine as a patch release, based on our current diff with 0.4.0. Would you like to do the release this time around @ssanderson? I can add you on PyPI as well.

ssanderson · 2017-10-25T18:58:21Z

@rgbkrk sure, I can do the release. Is there any special process that cloudpickle follows for releases besides setup.py sdist bdist_wheel upload from a recent pip?

rgbkrk · 2017-10-25T19:00:46Z

Beyond the setup.py uploading (for both Python2 and 3), I make sure to make a tag and then add some release notes to the GitHub releases page. We could really use some standardization of the releasing process. This project predates my current versioneer setup I put on all other packages.

holdenk · 2017-10-25T19:04:15Z

I know it’s post merge but just wanted to say from the Spark perspective dropping 2.6 is a-ok (we’re in the process of cleaning up the hacks and we’ve added for 2.6 now that we don’t support it either) :)

rgbkrk · 2017-10-25T19:04:31Z

I assumed your user was ssanderson based on your python-interface package. We could also add on @llllllllll if he's up to reducing our bus factor.

rgbkrk · 2017-10-25T19:10:34Z

@holdenk Luckily for now we skirted the issue of dropping support with @ssanderson's workarounds. 😄 I'll register that as a vote that we could turn around and drop 2.6 support for the next release 😈

## What changes were proposed in this pull request? The version of cloudpickle in PySpark was close to version 0.4.0 with some additional backported fixes and some minor additions for Spark related things. This update removes Spark related changes and matches cloudpickle [v0.4.3](https://github.com/cloudpipe/cloudpickle/releases/tag/v0.4.3): Changes by updating to 0.4.3 include: * Fix pickling of named tuples cloudpipe/cloudpickle#113 * Built in type constructors for PyPy compatibility [here](cloudpipe/cloudpickle@d84980c) * Fix memoryview support cloudpipe/cloudpickle#122 * Improved compatibility with other cloudpickle versions cloudpipe/cloudpickle#128 * Several cleanups cloudpipe/cloudpickle#121 and [here](cloudpipe/cloudpickle@c91aaf1) * [MRG] Regression on pickling classes from the __main__ module cloudpipe/cloudpickle#149 * BUG: Handle instance methods of builtin types cloudpipe/cloudpickle#154 * Fix <span>#</span>129 : do not silence RuntimeError in dump() cloudpipe/cloudpickle#153 ## How was this patch tested? Existing pyspark.tests using python 2.7.14, 3.5.2, 3.6.3 Author: Bryan Cutler <cutlerb@gmail.com> Closes apache#20373 from BryanCutler/pyspark-update-cloudpickle-42-SPARK-23159.

2.0.0 ===== - Python 3.5 is no longer supported. - Support for registering modules to be serialised by value. This allows code defined in local modules to be serialised and executed remotely without those local modules installed on the remote machine. ([PR #417](cloudpipe/cloudpickle#417)) - Fix a side effect altering dynamic modules at pickling time. ([PR #426](cloudpipe/cloudpickle#426)) - Support for pickling type annotations on Python 3.10 as per [PEP 563]( https://www.python.org/dev/peps/pep-0563/) ([PR #400](cloudpipe/cloudpickle#400)) - Stricter parametrized type detection heuristics in _is_parametrized_type_hint to limit false positives. ([PR #409](cloudpipe/cloudpickle#409)) - Support pickling / depickling of OrderedDict KeysView, ValuesView, and ItemsView, following similar strategy for vanilla Python dictionaries. ([PR #423](cloudpipe/cloudpickle#423)) - Suppressed a source of non-determinism when pickling dynamically defined functions and handles the deprecation of co_lnotab in Python 3.10+. ([PR #428](cloudpipe/cloudpickle#428)) 1.6.0 ===== - `cloudpickle`'s pickle.Pickler subclass (currently defined as `cloudpickle.cloudpickle_fast.CloudPickler`) can and should now be accessed as `cloudpickle.Pickler`. This is the only officially supported way of accessing it. ([issue #366](cloudpipe/cloudpickle#366)) - `cloudpickle` now supports pickling `dict_keys`, `dict_items` and `dict_values`. ([PR #384](cloudpipe/cloudpickle#384)) 1.5.0 ===== - Fix a bug causing cloudpickle to crash when pickling dynamically created, importable modules. ([issue #360](cloudpipe/cloudpickle#354)) - Add optional dependency on `pickle5` to get improved performance on Python 3.6 and 3.7. ([PR #370](cloudpipe/cloudpickle#370)) - Internal refactoring to ease the use of `pickle5` in cloudpickle for Python 3.6 and 3.7. ([PR #368](cloudpipe/cloudpickle#368)) 1.4.1 ===== - Fix incompatibilities between cloudpickle 1.4.0 and Python 3.5.0/1/2 introduced by the new support of cloudpickle for pickling typing constructs. ([issue #360](cloudpipe/cloudpickle#360)) - Restore compat with loading dynamic classes pickled with cloudpickle version 1.2.1 that would reference the `types.ClassType` attribute. ([PR #359](cloudpipe/cloudpickle#359)) 1.4.0 ===== **This version requires Python 3.5 or later** - cloudpickle can now all pickle all constructs from the ``typing`` module and the ``typing_extensions`` library in Python 3.5+ ([PR #318](cloudpipe/cloudpickle#318)) - Stop pickling the annotations of a dynamic class for Python < 3.6 (follow up on #276) ([issue #347](cloudpipe/cloudpickle#347)) - Fix a bug affecting the pickling of dynamic `TypeVar` instances on Python 3.7+, and expand the support for pickling `TypeVar` instances (dynamic or non-dynamic) to Python 3.5-3.6 ([PR #350](cloudpipe/cloudpickle#350)) - Add support for pickling dynamic classes subclassing `typing.Generic` instances on Python 3.7+ ([PR #351](cloudpipe/cloudpickle#351)) 1.3.0 ===== - Fix a bug affecting dynamic modules occuring with modified builtins ([issue #316](cloudpipe/cloudpickle#316)) - Fix a bug affecting cloudpickle when non-modules objects are added into sys.modules ([PR #326](cloudpipe/cloudpickle#326)). - Fix a regression in cloudpickle and python3.8 causing an error when trying to pickle property objects. ([PR #329](cloudpipe/cloudpickle#329)). - Fix a bug when a thread imports a module while cloudpickle iterates over the module list ([PR #322](cloudpipe/cloudpickle#322)). - Add support for out-of-band pickling (Python 3.8 and later). https://docs.python.org/3/library/pickle.html#example ([issue #308](cloudpipe/cloudpickle#308)) - Fix a side effect that would redefine `types.ClassTypes` as `type` when importing cloudpickle. ([issue #337](cloudpipe/cloudpickle#337)) - Fix a bug affecting subclasses of slotted classes. ([issue #311](cloudpipe/cloudpickle#311)) - Dont pickle the abc cache of dynamically defined classes for Python 3.6- (This was already the case for python3.7+) ([issue #302](cloudpipe/cloudpickle#302)) 1.2.2 ===== - Revert the change introduced in ([issue #276](cloudpipe/cloudpickle#276)) attempting to pickle functions annotations for Python 3.4 to 3.6. It is not possible to pickle complex typing constructs for those versions (see [issue #193]( cloudpipe/cloudpickle#193)) - Fix a bug affecting bound classmethod saving on Python 2. ([issue #288](cloudpipe/cloudpickle#288)) - Add support for pickling "getset" descriptors ([issue #290](cloudpipe/cloudpickle#290)) 1.2.1 ===== - Restore (partial) support for Python 3.4 for downstream projects that have LTS versions that would benefit from cloudpickle bug fixes. 1.2.0 ===== - Leverage the C-accelerated Pickler new subclassing API (available in Python 3.8) in cloudpickle. This allows cloudpickle to pickle Python objects up to 30 times faster. ([issue #253](cloudpipe/cloudpickle#253)) - Support pickling of classmethod and staticmethod objects in python2. arguments. ([issue #262](cloudpipe/cloudpickle#262)) - Add support to pickle type annotations for Python 3.5 and 3.6 (pickling type annotations was already supported for Python 3.7, Python 3.4 might also work but is no longer officially supported by cloudpickle) ([issue #276](cloudpipe/cloudpickle#276)) - Internal refactoring to proactively detect dynamic functions and classes when pickling them. This refactoring also yields small performance improvements when pickling dynamic classes (~10%) ([issue #273](cloudpipe/cloudpickle#273)) 1.1.1 ===== - Minor release to fix a packaging issue (Markdown formatting of the long description rendered on pypi.org). The code itself is the same as 1.1.0. 1.1.0 ===== - Support the pickling of interactively-defined functions with positional-only arguments. ([issue #266](cloudpipe/cloudpickle#266)) - Track the provenance of dynamic classes and enums so as to preseve the usual `isinstance` relationship between pickled objects and their original class defintions. ([issue #246](cloudpipe/cloudpickle#246)) 1.0.0 ===== - Fix a bug making functions with keyword-only arguments forget the default values of these arguments after being pickled. ([issue #264](cloudpipe/cloudpickle#264)) 0.8.1 ===== - Fix a bug (already present before 0.5.3 and re-introduced in 0.8.0) affecting relative import instructions inside depickled functions ([issue #254](cloudpipe/cloudpickle#254)) 0.8.0 ===== - Add support for pickling interactively defined dataclasses. ([issue #245](cloudpipe/cloudpickle#245)) - Global variables referenced by functions pickled by cloudpickle are now unpickled in a new and isolated namespace scoped by the CloudPickler instance. This restores the (previously untested) behavior of cloudpickle prior to changes done in 0.5.4 for functions defined in the `__main__` module, and 0.6.0/1 for other dynamic functions. 0.7.0 ===== - Correctly serialize dynamically defined classes that have a `__slots__` attribute. ([issue #225](cloudpipe/cloudpickle#225)) 0.6.1 ===== - Fix regression in 0.6.0 which breaks the pickling of local function defined in a module, making it impossible to access builtins. ([issue #211](cloudpipe/cloudpickle#211)) 0.6.0 ===== - Ensure that unpickling a function defined in a dynamic module several times sequentially does not reset the values of global variables. ([issue #187](cloudpipe/cloudpickle#205)) - Restrict the ability to pickle annotations to python3.7+ ([issue #193]( cloudpipe/cloudpickle#193) and [issue #196]( cloudpipe/cloudpickle#196)) - Stop using the deprecated `imp` module under Python 3. ([issue #207](cloudpipe/cloudpickle#207)) - Fixed pickling issue with singleton types `NoneType`, `type(...)` and `type(NotImplemented)` ([issue #209](cloudpipe/cloudpickle#209)) 0.5.6 ===== - Ensure that unpickling a locally defined function that accesses the global variables of a module does not reset the values of the global variables if they are already initialized. ([issue #187](cloudpipe/cloudpickle#187)) 0.5.5 ===== - Fixed inconsistent version in `cloudpickle.__version__`. 0.5.4 ===== - Fixed a pickling issue for ABC in python3.7+ ([issue #180]( cloudpipe/cloudpickle#180)). - Fixed a bug when pickling functions in `__main__` that access global variables ([issue #187]( cloudpipe/cloudpickle#187)). 0.5.3 ===== - Fixed a crash in Python 2 when serializing non-hashable instancemethods of built-in types ([issue #144](cloudpipe/cloudpickle#144)). - itertools objects can also pickled ([PR #156](cloudpipe/cloudpickle#156)). - `logging.RootLogger` can be also pickled ([PR #160](cloudpipe/cloudpickle#160)). 0.5.2 ===== - Fixed a regression: `AttributeError` when loading pickles that hold a reference to a dynamically defined class from the `__main__` module. ([issue #131]( cloudpipe/cloudpickle#131)). - Make it possible to pickle classes and functions defined in faulty modules that raise an exception when trying to look-up their attributes by name. 0.5.1 ===== - Fixed `cloudpickle.__version__`. 0.5.0 ===== - Use `pickle.HIGHEST_PROTOCOL` by default. 0.4.4 ===== - `logging.RootLogger` can be also pickled ([PR #160](cloudpipe/cloudpickle#160)). 0.4.3 ===== - Fixed a regression: `AttributeError` when loading pickles that hold a reference to a dynamically defined class from the `__main__` module. ([issue #131]( cloudpipe/cloudpickle#131)). - Fixed a crash in Python 2 when serializing non-hashable instancemethods of built-in types. ([issue #144](cloudpipe/cloudpickle#144)) 0.4.2 ===== - Restored compatibility with pickles from 0.4.0. - Handle the `func.__qualname__` attribute. 0.4.1 ===== - Fixed a crash when pickling dynamic classes whose `__dict__` attribute was defined as a [`property`](https://docs.python.org/3/library/functions.html#property). Most notably, this affected dynamic [namedtuples](https://docs.python.org/2/library/collections.html#namedtuple-factory-function-for-tuples-with-named-fields) in Python 2. (cloudpipe/cloudpickle#113) - Cloudpickle now preserves the `__module__` attribute of functions (cloudpipe/cloudpickle#118). - Fixed a crash when pickling modules that don't have a `__package__` attribute (cloudpipe/cloudpickle#116). 0.4.0 ===== * Fix functions with empty cells * Allow pickling Logger objects * Fix crash when pickling dynamic class cycles * Ignore "None" mdoules added to sys.modules * Support WeakSets and ABCMeta instances * Remove non-standard `__transient__` support * Catch exception from `pickle.whichmodule()` 0.3.1 ===== * Fix version information and ship a changelog 0.3.0 ===== * Import submodules accessed by pickled functions * Support recursive functions inside closures * Fix `ResourceWarnings` and `DeprecationWarnings` * Assume modules with `__file__` attribute are not dynamic 0.2.2 ===== * Support Python 3.6 * Support Tornado Coroutines * Support builtin methods

ssanderson commented Aug 5, 2017

View reviewed changes

ssanderson mentioned this pull request Aug 10, 2017

Fail to unpickle namedtuple #111

Closed

robertnishihara reviewed Aug 15, 2017

View reviewed changes

Scott Sanderson added 2 commits August 30, 2017 19:03

BUG: Fix bug pickling namedtuple.

8675793

Fixes a crash when trying to set `__dict__` onto a type when `__dict__` is a property.

MAINT: Fix namedtuple tests.

b3f2af9

ssanderson force-pushed the fix-namedtuple branch from 7ccde35 to b3f2af9 Compare August 30, 2017 23:03

MAINT: Handle builtin type __new__ attrs.

28e9e7e

ssanderson force-pushed the fix-namedtuple branch from ef4153e to 28e9e7e Compare August 30, 2017 23:56

MAINT: assertIs doesn't exist in 2.6.

a953a59

denismakogon approved these changes Oct 21, 2017

View reviewed changes

robertnishihara approved these changes Oct 22, 2017

View reviewed changes

Scott Sanderson added 2 commits October 22, 2017 17:03

BUG: Hit the builtin type cache for any function.

70e3a46

On PyPy, built-in type constructors aren't necessarily builtin_function_or_method instances, so we need to check for them in `save_function` rather than just in `save_builtin_function`.

TEST: assertIsInstance doesn't exist on 2.6.

6fb05ca

rgbkrk merged commit bda339f into master Oct 25, 2017

rgbkrk deleted the fix-namedtuple branch October 25, 2017 18:53

pyup-bot mentioned this pull request Oct 27, 2017

Update cloudpickle to 0.4.1 zenotech/zCFDSuperBuild#94

Merged

robertnishihara mentioned this pull request Oct 27, 2017

Pin cloudpickle version to 0.3 in travis tests. ray-project/ray#834

Merged

ssanderson mentioned this pull request Dec 28, 2017

Failure on unhashable functions in Python 2 #144

Closed

BryanCutler mentioned this pull request Jan 23, 2018

[SPARK-23159][PYTHON] Update cloudpickle to v0.4.3 apache/spark#20373

Closed

HyukjinKwon mentioned this pull request Jan 29, 2018

BUG: Handle instance methods of builtin types. #145

Merged

HyukjinKwon mentioned this pull request Apr 26, 2018

[SPARK-22674][PYTHON] Removed the namedtuple pickling patch apache/spark#21157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix bug pickling namedtuple. #113

BUG: Fix bug pickling namedtuple. #113

ssanderson commented Aug 5, 2017

ssanderson Aug 5, 2017

ssanderson Aug 5, 2017

rgbkrk commented Aug 7, 2017

rgbkrk commented Aug 9, 2017

ssanderson commented Aug 9, 2017 via email

rgbkrk commented Aug 9, 2017

robertnishihara Aug 15, 2017

codecov-io commented Aug 30, 2017 •

edited

Loading

rgbkrk commented Oct 22, 2017

mrocklin commented Oct 22, 2017

ssanderson commented Oct 22, 2017

ssanderson commented Oct 22, 2017

ssanderson commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

ssanderson commented Oct 25, 2017

rgbkrk commented Oct 25, 2017 •

edited

Loading

holdenk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

		@@ -652,6 +652,15 @@ def __init__(self, x):

		self.assertEqual(set(weakset), set([depickled1, depickled2]))

		def test_namedtuple(self):

BUG: Fix bug pickling namedtuple. #113

BUG: Fix bug pickling namedtuple. #113

Conversation

ssanderson commented Aug 5, 2017

ssanderson Aug 5, 2017

Choose a reason for hiding this comment

ssanderson Aug 5, 2017

Choose a reason for hiding this comment

rgbkrk commented Aug 7, 2017

rgbkrk commented Aug 9, 2017

ssanderson commented Aug 9, 2017 via email

rgbkrk commented Aug 9, 2017

robertnishihara Aug 15, 2017

Choose a reason for hiding this comment

codecov-io commented Aug 30, 2017 • edited Loading

Codecov Report

rgbkrk commented Oct 22, 2017

mrocklin commented Oct 22, 2017

ssanderson commented Oct 22, 2017

ssanderson commented Oct 22, 2017

ssanderson commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

ssanderson commented Oct 25, 2017

rgbkrk commented Oct 25, 2017 • edited Loading

holdenk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

rgbkrk commented Oct 25, 2017

codecov-io commented Aug 30, 2017 •

edited

Loading

rgbkrk commented Oct 25, 2017 •

edited

Loading