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

Optionally use pickle5 (Redux) #370

Merged
merged 98 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from 94 commits
Commits
Show all changes
98 commits
Select commit Hold shift + click to select a range
a860509
backport python3.8 save_reduce into old CloudPickler
pierreglaser May 14, 2020
ee722f1
python 3.8 compat
pierreglaser May 14, 2020
d69ca86
use the same Cloudpickler class for all Pythons
pierreglaser May 14, 2020
6264641
increase flake8 max complexity
pierreglaser May 14, 2020
dc9ba84
unused functions and imports
pierreglaser May 14, 2020
d5ed4d9
pypy compat
pierreglaser May 14, 2020
c44ad88
be pickle5 friendly
pierreglaser May 14, 2020
fd67f4b
duplicate top-level functions
pierreglaser May 14, 2020
9740b22
increase flake8 max-complexity
pierreglaser May 14, 2020
c657fe1
install tornado in the CI
pierreglaser May 14, 2020
1f2410f
Add `compat`
jakirkham May 21, 2020
ebaf7f6
Use `compat` to provide `pickle` or `pickle5`
jakirkham May 21, 2020
1d6abbd
Import `pickle` from `compat` in tests
jakirkham May 21, 2020
fb34d27
Add pickle5 to the dev requirements
jakirkham May 22, 2020
a15782f
Always remove `_abc_impl` from `clsdict`
jakirkham May 22, 2020
35e39d0
Keep all abstract base class elements
jakirkham May 22, 2020
a03a1ab
call typing reducers inside reducer_override
pierreglaser May 29, 2020
cdaeec5
revert older temptative fixes
pierreglaser May 29, 2020
95fdce0
skip numpy + pickle5 test on Python 3.5
pierreglaser May 29, 2020
38ed5f5
don't try to install pickle5 on PYPY
pierreglaser May 29, 2020
5334e79
Simplify `version_info` check
jakirkham Jun 10, 2020
abbb978
Update cloudpickle/cloudpickle.py
pierreglaser Jun 10, 2020
eb536de
Merge 'pierreglaser/dedup-reducers' into 'jakirkham/opt_use_pickle5_r…
jakirkham Jun 10, 2020
53111d3
Tweak pickle5 imports
jakirkham Jun 11, 2020
9d56632
test best-effort cloudpickle backward compat
pierreglaser Jun 14, 2020
9e92413
debug ci not launching
pierreglaser Jun 14, 2020
043ff81
Merge branch 'master' into dedup-reducers
pierreglaser Jun 14, 2020
09d9e33
solve merge conflics mistakes
pierreglaser Jun 14, 2020
f74372a
generate old pickle files during CI
pierreglaser Jun 14, 2020
829b919
fixup! generate old pickle files during CI
pierreglaser Jun 14, 2020
f693401
fix pathlib/open compat
pierreglaser Jun 14, 2020
46405e5
update python-nightly entry
pierreglaser Jun 14, 2020
f57df63
Merge pierreglaser/dedup-reducers into jakirkham/opt_use_pickle5_redux
jakirkham Jun 14, 2020
df6fa9b
fix ci script
pierreglaser Jun 15, 2020
20a522b
fix variable reference in ci script
pierreglaser Jun 17, 2020
3908dfe
Merge branch 'master' into dedup-reducers
pierreglaser Jun 17, 2020
e5deaf6
restore backward compat with cloudpickle master
pierreglaser Jun 17, 2020
ff2baa9
add deprecation warnings inside old reconstructors
pierreglaser Jun 17, 2020
13a76f9
always use up-to-date copyreg
pierreglaser Jun 17, 2020
393b312
Merge branch 'dedup-reducers' of github.com:pierreglaser/cloudpickle …
pierreglaser Jun 17, 2020
52746b9
don't (always) catch deprecation warnings
pierreglaser Jun 17, 2020
b531b14
fixup! don't (always) catch deprecation warnings
pierreglaser Jun 17, 2020
e85ab4d
debug ci
pierreglaser Jun 17, 2020
2a82a41
fixup! debug ci
pierreglaser Jun 17, 2020
21c82b9
fixup! debug ci
pierreglaser Jun 17, 2020
4fb4ca5
fixup! debug ci
pierreglaser Jun 18, 2020
5909c38
remove debug leftovers
pierreglaser Jun 21, 2020
2822390
separate pickles per python version
pierreglaser Jun 21, 2020
d239943
fixup! separate pickles per python version
pierreglaser Jun 21, 2020
8deef74
fixup! separate pickles per python version
pierreglaser Jun 21, 2020
bb3436f
add some pickles generated with cloudpickle 1.4.1
pierreglaser Jun 21, 2020
884ee89
stop generating pickles inside the CI
pierreglaser Jun 21, 2020
3b98492
fix use correct pypy version
pierreglaser Jun 21, 2020
0469528
Merge pierreglaser/dedup-reducers into jakirkham/opt_use_pickle5_redux
jakirkham Jun 22, 2020
371d015
Remove commented lines
ogrisel Jun 29, 2020
d661ce6
tornado is already in dev-requirements.txt
ogrisel Jun 30, 2020
4de14b2
Test nested function in old_pickles
ogrisel Jun 30, 2020
a0f8d73
Skip failing macos + pypy3
ogrisel Jun 30, 2020
ca07fce
Trigger CI
ogrisel Jun 30, 2020
96fe5c0
copy cloudpickle_fast.py inside ray source
pierreglaser Jun 30, 2020
78602e2
fixup! copy cloudpickle_fast.py inside ray source
pierreglaser Jun 30, 2020
dad2938
fixup! copy cloudpickle_fast.py inside ray source
pierreglaser Jun 30, 2020
4dcbf3e
update vendored ray to more recent version
pierreglaser Jun 30, 2020
677a114
install ray outside of cloudpickle
pierreglaser Jun 30, 2020
cd7cd1a
don't try to test deleted ray tests
pierreglaser Jun 30, 2020
8eba950
fixup! don't try to test deleted ray tests
pierreglaser Jun 30, 2020
bff0786
Add `compat`
jakirkham May 21, 2020
fccb9e3
Use `compat` to provide `pickle` or `pickle5`
jakirkham May 21, 2020
0d11d66
Import `pickle` from `compat` in tests
jakirkham May 21, 2020
0522cfb
Add pickle5 to the dev requirements
jakirkham May 22, 2020
149b01e
Always remove `_abc_impl` from `clsdict`
jakirkham May 22, 2020
695cbb8
Keep all abstract base class elements
jakirkham May 22, 2020
adc1220
call typing reducers inside reducer_override
pierreglaser May 29, 2020
71dcd2a
revert older temptative fixes
pierreglaser May 29, 2020
e4fc3a0
skip numpy + pickle5 test on Python 3.5
pierreglaser May 29, 2020
e41b4dd
don't try to install pickle5 on PYPY
pierreglaser May 29, 2020
4688643
Simplify `version_info` check
jakirkham Jun 10, 2020
5bfb9f3
Tweak pickle5 imports
jakirkham Jun 11, 2020
794cd9d
empty commit to trigger the ci
pierreglaser Jun 30, 2020
b9ccea7
add compat to vendored cloudpickle in ray
pierreglaser Jun 30, 2020
afec159
remove unused import
pierreglaser Jun 30, 2020
1dc8dd8
use latest pickle5 with ray
pierreglaser Jun 30, 2020
9502999
restor CloudPickler's dispatch attribute
pierreglaser Jun 30, 2020
6530ff6
use C-implemented pickler if pickle5 is installed
pierreglaser Jun 30, 2020
7aabd2a
mention the overloaded meaning of `dispatch`
pierreglaser Jun 30, 2020
4d33877
Merge pierreglaser/dedup-reducers into jakirkham/opt_use_pickle5_redux
jakirkham Jun 30, 2020
5800bb6
Merge pierreglaser/pickle5-tmp into jakirkham/opt_use_pickle5_redux
jakirkham Jun 30, 2020
4e1bc25
Merge cloudpipe/master into jakirkham/opt_use_pickle5_redux
jakirkham Jun 30, 2020
cc5efb2
Drop `pickle5` from CI (already in dev reqs)
jakirkham Jun 30, 2020
ec3468e
Require NumPy 1.18.5 for testing
jakirkham Jun 30, 2020
d588774
Require pickle5 0.0.11
jakirkham Jun 30, 2020
7b3f1a7
Include NumPy test on Python 3.5
jakirkham Jun 30, 2020
8178a2c
Add blank line
jakirkham Jun 30, 2020
dc8bd28
Revert "Add blank line"
jakirkham Jun 30, 2020
8a890f5
Fix typo
jakirkham Jun 30, 2020
5cdb5e1
Update cloudpickle/cloudpickle_fast.py
pierreglaser Jul 1, 2020
268778b
CI trigger
pierreglaser Jul 1, 2020
f17b31a
Update CHANGES.md
ogrisel Jul 1, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ jobs:
git clone https://github.com/ray-project/ray.git ../ray
cp -R ../ray/python/ray/tests $PROJECT_DIR/tests
cp cloudpickle/cloudpickle.py $PROJECT_DIR/cloudpickle/cloudpickle.py
cp cloudpickle/compat.py $PROJECT_DIR/cloudpickle/compat.py
cp cloudpickle/cloudpickle_fast.py $PROJECT_DIR/cloudpickle/cloudpickle_fast.py
- name: Test the downstream project
run: |
Expand Down
2 changes: 1 addition & 1 deletion cloudpickle/cloudpickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import builtins
import dis
import opcode
import pickle
import platform
import sys
import types
Expand All @@ -55,6 +54,7 @@
import typing
import warnings

from .compat import pickle
from typing import Generic, Union, Tuple, Callable
from pickle import _getattribute
from importlib._bootstrap import _find_spec
Expand Down
24 changes: 19 additions & 5 deletions cloudpickle/cloudpickle_fast.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import io
import itertools
import logging
import pickle
import sys
import struct
import types
Expand All @@ -25,6 +24,7 @@
from enum import Enum
from collections import ChainMap

from .compat import pickle, Pickler
from .cloudpickle import (
_extract_code_globals, _BUILTIN_TYPE_NAMES, DEFAULT_PROTOCOL,
_find_imported_submodules, _get_cell_contents, _is_importable,
Expand All @@ -37,8 +37,8 @@

)

if sys.version_info >= (3, 8) and not PYPY:
from _pickle import Pickler

if pickle.HIGHEST_PROTOCOL >= 5 and not PYPY:
# Shorthands similar to pickle.dump/pickle.dumps

def dump(obj, file, protocol=None, buffer_callback=None):
Expand Down Expand Up @@ -73,8 +73,6 @@ def dumps(obj, protocol=None, buffer_callback=None):
return file.getvalue()

else:
from pickle import _Pickler as Pickler

# Shorthands similar to pickle.dump/pickle.dumps
def dump(obj, file, protocol=None):
"""Serialize obj as bytes streamed into file
Expand Down Expand Up @@ -551,6 +549,17 @@ def dump(self, obj):
raise

if pickle.HIGHEST_PROTOCOL >= 5:
# `Cloudpickle.dispatch` is only left for backward compatibility - note
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
# that when using protocol 5, `CloudPickler.dispatch` is not an
# extension of `Pickler.dispatch` dictionary, because CloudPickler
# subclasses the C-implemented Pickler, which does not expose a
# `dispatch` attribute. Earlier version of the protocol 5 CloudPickler
jakirkham marked this conversation as resolved.
Show resolved Hide resolved
# used `CloudPickler.dispatch` as a class-level attribute storing all
# reducers implemented by cloudpickle, but the attribute name was not a
# great choice given the meaning of `Cloudpickler.dispatch` when
# `CloudPickler` extends the pure-python pickler.
dispatch = dispatch_table

# Implementation of the reducer_override callback, in order to
# efficiently serialize dynamic functions and classes by subclassing
# the C-implemented Pickler.
Expand Down Expand Up @@ -604,6 +613,11 @@ def reducer_override(self, obj):
reducers, such as Exceptions. See
https://github.com/cloudpipe/cloudpickle/issues/248
"""
if sys.version_info[:2] < (3, 7) and _is_parametrized_type_hint(obj): # noqa # pragma: no branch
return (
_create_parametrized_type_hint,
parametrized_type_hint_getinitargs(obj)
)
t = type(obj)
try:
is_anyclass = issubclass(t, type)
Expand Down
13 changes: 13 additions & 0 deletions cloudpickle/compat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import sys


if sys.version_info < (3, 8):
try:
import pickle5 as pickle # noqa: F401
from pickle5 import Pickler # noqa: F401
except ImportError:
import pickle # noqa: F401
from pickle import _Pickler as Pickler # noqa: F401
else:
import pickle # noqa: F401
from _pickle import Pickler # noqa: F401
4 changes: 3 additions & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ flake8
pytest
pytest-cov
psutil
# To test on older Python versions
pickle5 >=0.0.11 ; python_version <= '3.7' and python_implementation == 'CPython'
# To be able to test tornado coroutines
tornado
# To be able to test numpy specific things
# but do not build numpy from source on Python nightly
numpy; python_version <= '3.8'
numpy >=1.18.5; python_version <= '3.8'
Copy link
Member

Choose a reason for hiding this comment

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

so we don't have to skip numpy + Python 3.5 after all? nice :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed :) PR ( numpy/numpy#16439 ) added Python 3.5 support

# Code coverage uploader for Travis:
codecov
coverage
Expand Down
2 changes: 1 addition & 1 deletion tests/cloudpickle_file_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import unicode_literals

import os
import pickle
import shutil
import sys
import tempfile
Expand All @@ -10,6 +9,7 @@
import pytest

import cloudpickle
from cloudpickle.compat import pickle


class CloudPickleFileTests(unittest.TestCase):
Expand Down
16 changes: 9 additions & 7 deletions tests/cloudpickle_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import logging
import math
from operator import itemgetter, attrgetter
import pickle
import platform
import random
import shutil
Expand Down Expand Up @@ -43,6 +42,7 @@
tornado = None

import cloudpickle
from cloudpickle.compat import pickle
from cloudpickle.cloudpickle import _is_importable
from cloudpickle.cloudpickle import _make_empty_cell, cell_set
from cloudpickle.cloudpickle import _extract_class_dict, _whichmodule
Expand Down Expand Up @@ -521,7 +521,7 @@ def test_module_locals_behavior(self):
pickled_func_path = os.path.join(self.tmpdir, 'local_func_g.pkl')

child_process_script = '''
import pickle
from cloudpickle.compat import pickle
Copy link
Member

Choose a reason for hiding this comment

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

using cloudpickle.compat.pickle or pickle should be equivalent in the load case right?

import gc
with open("{pickled_func_path}", 'rb') as f:
func = pickle.load(f)
Expand Down Expand Up @@ -606,7 +606,7 @@ def test_load_dynamic_module_in_grandchild_process(self):
child_process_module_file = os.path.join(
self.tmpdir, 'dynamic_module_from_child_process.pkl')
child_process_script = '''
import pickle
from cloudpickle.compat import pickle
import textwrap

import cloudpickle
Expand All @@ -626,7 +626,7 @@ def test_load_dynamic_module_in_grandchild_process(self):

# The script ran by the process created by the child process
child_of_child_process_script = """ '''
import pickle
from cloudpickle.compat import pickle
with open('{child_process_module_file}','rb') as fid:
mod = pickle.load(fid)
''' """
Expand Down Expand Up @@ -681,7 +681,7 @@ def my_small_function(x, y):
assert b'math' not in b

def test_module_importability(self):
import pickle # decouple this test from global imports
from cloudpickle.compat import pickle
import os.path
import distutils
import distutils.ccompiler
Expand Down Expand Up @@ -1008,7 +1008,8 @@ def example():

# choose "subprocess" rather than "multiprocessing" because the latter
# library uses fork to preserve the parent environment.
command = ("import pickle, base64; "
command = ("import base64; "
"from cloudpickle.compat import pickle; "
Copy link
Member

Choose a reason for hiding this comment

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

ditto: cloudpickle.compat.pickle and pickle are interchangeable in this situation right?

Copy link
Member Author

Choose a reason for hiding this comment

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

In both of these test cases, we are using the highest supported protocol with cloudpickle, which is protocol 5, to produce the pickled data. So we wound up needing to change these since we are trying to load the pickled data and then need to use pickle5 when pickle doesn't have protocol 5 support. We could alternatively skip the test, restrict the protocol based on pickle.HIGHEST_PROTOCOL, or something else. We could also leave as-is. Some options to consider 🙂

Copy link
Member

Choose a reason for hiding this comment

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

In this case I agree. Thanks.

"pickle.loads(base64.b32decode('" +
base64.b32encode(s).decode('ascii') +
"'))()")
Expand All @@ -1029,7 +1030,8 @@ def example():

s = cloudpickle.dumps(example, protocol=self.protocol)

command = ("import pickle, base64; "
command = ("import base64; "
"from cloudpickle.compat import pickle; "
"pickle.loads(base64.b32decode('" +
base64.b32encode(s).decode('ascii') +
"'))()")
Expand Down
3 changes: 2 additions & 1 deletion tests/testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
import tempfile
import base64
from subprocess import Popen, check_output, PIPE, STDOUT, CalledProcessError
from pickle import loads
from cloudpickle.compat import pickle
from contextlib import contextmanager
from concurrent.futures import ProcessPoolExecutor

import psutil
from cloudpickle import dumps
from subprocess import TimeoutExpired

loads = pickle.loads
TIMEOUT = 60
TEST_GLOBALS = "a test value"

Expand Down