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 cloudpickle incompatibilities on early Python 3.5 versions #361

Merged
merged 33 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
fcbce90
fix basic incompatibilities with old Python3.5
pierreglaser Apr 28, 2020
3fa3227
fix pickling of type hints in Python 3.5.[0-2]
pierreglaser Apr 28, 2020
732343e
modify typing test to account for Python 3.5.[0-2]
pierreglaser Apr 28, 2020
3adc5be
fixup! fix pickling of type hints in Python 3.5.[0-2]
pierreglaser Apr 28, 2020
d2b8aa6
fixup! fix pickling of type hints in Python 3.5.[0-2]
pierreglaser Apr 28, 2020
96c1d2f
update changelog
pierreglaser Apr 28, 2020
7996513
fix mistake in parametrized type hint checker
pierreglaser Apr 28, 2020
f164a29
try to test cloudpickle agains Python 3.5.0 in CI
pierreglaser Apr 28, 2020
a7e521f
try to use conda python in the CI
pierreglaser Apr 28, 2020
2c13f90
Merge branch 'master' into cloudpickle-py350
ogrisel Apr 28, 2020
4af5e8d
fixup! try to use conda python in the CI
pierreglaser Apr 28, 2020
47d981a
Merge branch 'cloudpickle-py350' of github.com:pierreglaser/cloudpick…
pierreglaser Apr 28, 2020
2c9e644
debug miniconda
pierreglaser Apr 28, 2020
d5e0586
fixup! debug miniconda
pierreglaser Apr 28, 2020
f50f87f
fixup! fixup! debug miniconda
pierreglaser Apr 28, 2020
726d86d
fixup! fixup! debug miniconda
pierreglaser Apr 28, 2020
0d834ce
fixup! fixup! fixup! debug miniconda
pierreglaser Apr 28, 2020
405b99a
a few last tries
pierreglaser Apr 28, 2020
b1eca25
fixup! a few last tries
pierreglaser Apr 28, 2020
abd46d7
I think I get it
pierreglaser Apr 28, 2020
1fb09a0
rollback CI changes
pierreglaser Apr 28, 2020
736127c
fix some Python version check
pierreglaser Apr 28, 2020
6e3621d
this cannot be done using version checks
pierreglaser Apr 28, 2020
ec41dee
this cannot be done using version checks
pierreglaser Apr 28, 2020
ae643fb
fix typing_extension tests for early Python 3.5
pierreglaser Apr 28, 2020
a9dadd6
revert TypeVar tracking
pierreglaser Apr 28, 2020
acd23e4
fix sys version checks
pierreglaser Apr 28, 2020
e573c37
add a few explanations
pierreglaser Apr 28, 2020
c9c8b46
Update cloudpickle/cloudpickle.py
ogrisel Apr 29, 2020
cf9e42e
Re-enable dynamic TypeVar tracking
ogrisel Apr 29, 2020
8c030a3
Apply suggestions from code review
ogrisel Apr 29, 2020
6359bc5
be more explicit when testing Final/Literal
pierreglaser Apr 29, 2020
e58c34b
Workaround for Python 3.5.3
ogrisel Apr 29, 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
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
1.4.1 (in development)
======================

- 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](https://github.com/cloudpipe/cloudpickle/issues/360))

- Restore compat with loading dynamic classes pickled with cloudpickle
version 1.2.1 that would reference the `types.ClassType` attribute.
([PR #359](https://github.com/cloudpipe/cloudpickle/pull/359))
Expand Down
80 changes: 67 additions & 13 deletions cloudpickle/cloudpickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
import typing
from enum import Enum

from typing import Generic, Union, Tuple, Callable, ClassVar
from typing import Generic, Union, Tuple, Callable
from pickle import _Pickler as Pickler
from pickle import _getattribute
from io import BytesIO
Expand All @@ -73,6 +73,11 @@
except ImportError:
_typing_extensions = Literal = Final = None

if sys.version_info >= (3, 5, 3):
from typing import ClassVar
else: # pragma: no cover
ClassVar = None


# cloudpickle is meant for inter process communication: we expect all
# communicating processes to run the same Python version hence we favor
Expand Down Expand Up @@ -432,10 +437,24 @@ def _extract_class_dict(cls):
if sys.version_info[:2] < (3, 7): # pragma: no branch
def _is_parametrized_type_hint(obj):
# This is very cheap but might generate false positives.
origin = getattr(obj, '__origin__', None) # typing Constructs
values = getattr(obj, '__values__', None) # typing_extensions.Literal
type_ = getattr(obj, '__type__', None) # typing_extensions.Final
return origin is not None or values is not None or type_ is not None
# general typing Constructs
is_typing = getattr(obj, '__origin__', None) is not None

# typing_extensions.Literal
is_litteral = getattr(obj, '__values__', None) is not None

# typing_extensions.Final
is_final = getattr(obj, '__type__', None) is not None

# typing.Union/Tuple for old Python 3.5
is_union = getattr(obj, '__union_params__', None) is not None
is_tuple = getattr(obj, '__tuple_params__', None) is not None
is_callable = (
getattr(obj, '__result__', None) is not None and
getattr(obj, '__args__', None) is not None
)
return any((is_typing, is_litteral, is_final, is_union, is_tuple,
is_callable))

def _create_parametrized_type_hint(origin, args):
return origin[args]
Expand Down Expand Up @@ -971,14 +990,40 @@ def _save_parametrized_type_hint(self, obj):
initargs = (Final, obj.__type__)
elif type(obj) is type(ClassVar):
initargs = (ClassVar, obj.__type__)
elif type(obj) in [type(Union), type(Tuple), type(Generic)]:
initargs = (obj.__origin__, obj.__args__)
elif type(obj) is type(Generic):
parameters = obj.__parameters__
if len(obj.__parameters__) > 0:
# in early Python 3.5, __parameters__ was sometimes
# preferred to __args__
initargs = (obj.__origin__, parameters)
else:
initargs = (obj.__origin__, obj.__args__)
elif type(obj) is type(Union):
if sys.version_info < (3, 5, 3): # pragma: no cover
initargs = (Union, obj.__union_params__)
else:
initargs = (Union, obj.__args__)
elif type(obj) is type(Tuple):
if sys.version_info < (3, 5, 3): # pragma: no cover
initargs = (Tuple, obj.__tuple_params__)
else:
initargs = (Tuple, obj.__args__)
elif type(obj) is type(Callable):
args = obj.__args__
if args[0] is Ellipsis:
initargs = (obj.__origin__, args)
if sys.version_info < (3, 5, 3): # pragma: no cover
args = obj.__args__
result = obj.__result__
if args != Ellipsis:
if isinstance(args, tuple):
args = list(args)
else:
args = [args]
else:
initargs = (obj.__origin__, (list(args[:-1]), args[-1]))
(*args, result) = obj.__args__
if len(args) == 1 and args[0] is Ellipsis:
args = Ellipsis
else:
args = list(args)
initargs = (Callable, (args, result))
else: # pragma: no cover
raise pickle.PicklingError(
"Cloudpickle Error: Unknown type {}".format(type(obj))
Expand Down Expand Up @@ -1301,14 +1346,23 @@ def _make_typevar(name, bound, constraints, covariant, contravariant,
name, *constraints, bound=bound,
covariant=covariant, contravariant=contravariant
)
return _lookup_class_or_track(class_tracker_id, tv)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you track TypeVar defintions anymore? This seems unrelated to the change to support early 3.5.x.

Doesn't this change break ant test?

Edit: I see you removed test_pickle_dynamic_typevar_tracking.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also won't this break unpickling objects pickled with cloudpickle 1.4.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

TyperVar instances are not weakreferable in Python 3.5.3..

Copy link
Member Author

Choose a reason for hiding this comment

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

Also won't this break unpickling objects pickled with cloudpickle 1.4.0?

We can always restore the class_tracker_id for backward compat.

Copy link
Contributor

Choose a reason for hiding this comment

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

TyperVar instances are not weakreferable in Python 3.5.3..

I re-ran the tests with old Python 3.5.x and they pass...

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum you are right they fail just for 3.5.3 ...

Copy link
Contributor

Choose a reason for hiding this comment

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

ok let me do a workaround for 3.5.3.

if class_tracker_id is not None:
return _lookup_class_or_track(class_tracker_id, tv)
else: # pragma: nocover
# Only for Python 3.5.3 compat.
return tv


def _decompose_typevar(obj):
try:
class_tracker_id = _get_or_create_tracker_id(obj)
except TypeError: # pragma: nocover
# TypeVar instances are not weakref-able in Python 3.5.3
class_tracker_id = None
return (
obj.__name__, obj.__bound__, obj.__constraints__,
obj.__covariant__, obj.__contravariant__,
_get_or_create_tracker_id(obj),
class_tracker_id,
)


Expand Down
79 changes: 59 additions & 20 deletions tests/cloudpickle_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,9 @@ def test_pickle_dynamic_typevar(self):
for attr in attr_list:
assert getattr(T, attr) == getattr(depickled_T, attr)

@pytest.mark.skipif(
sys.version_info[:3] == (3, 5, 3),
reason="TypeVar instances are not weakref-able in Python 3.5.3")
def test_pickle_dynamic_typevar_tracking(self):
T = typing.TypeVar("T")
T2 = subprocess_pickle_echo(T, protocol=self.protocol)
Expand Down Expand Up @@ -2081,20 +2084,32 @@ class C(typing.Generic[T]):

with subprocess_worker(protocol=self.protocol) as worker:

def check_generic(generic, origin, type_value):
def check_generic(generic, origin, type_value, use_args):
assert generic.__origin__ is origin
assert len(generic.__args__) == 1
assert generic.__args__[0] is type_value

assert len(origin.__orig_bases__) == 1
ob = origin.__orig_bases__[0]
assert ob.__origin__ is typing.Generic
if sys.version_info >= (3, 5, 3):
assert len(origin.__orig_bases__) == 1
ob = origin.__orig_bases__[0]
assert ob.__origin__ is typing.Generic
else: # Python 3.5.[0-1-2], pragma: no cover
assert len(origin.__bases__) == 1
ob = origin.__bases__[0]

if use_args:
assert len(generic.__args__) == 1
assert generic.__args__[0] is type_value
else:
assert len(generic.__parameters__) == 1
assert generic.__parameters__[0] is type_value
assert len(ob.__parameters__) == 1

return "ok"

assert check_generic(C[int], C, int) == "ok"
assert worker.run(check_generic, C[int], C, int) == "ok"
# backward-compat for old Python 3.5 versions that sometimes relies
# on __parameters__
use_args = getattr(C[int], '__args__', ()) != ()
assert check_generic(C[int], C, int, use_args) == "ok"
assert worker.run(check_generic, C[int], C, int, use_args) == "ok"

def test_locally_defined_class_with_type_hints(self):
with subprocess_worker(protocol=self.protocol) as worker:
Expand All @@ -2116,19 +2131,38 @@ def check_annotations(obj, expected_type):
assert check_annotations(obj, type_) == "ok"
assert worker.run(check_annotations, obj, type_) == "ok"

def test_generic_extensions(self):
def test_generic_extensions_literal(self):
typing_extensions = pytest.importorskip('typing_extensions')

objs = [
typing_extensions.Literal,
typing_extensions.Final,
typing_extensions.Literal['a'],
typing_extensions.Final[int],
def check_literal_equal(obj1, obj2):
assert obj1.__values__ == obj2.__values__
assert type(obj1) == type(obj2) == typing_extensions._LiteralMeta
literal_objs = [
typing_extensions.Literal, typing_extensions.Literal['a']
]
for obj in literal_objs:
depickled_obj = pickle_depickle(obj, protocol=self.protocol)
if sys.version_info[:3] >= (3, 5, 3):
assert depickled_obj == obj
else:
# __eq__ does not work for Literal objects in early Python 3.5
check_literal_equal(obj, depickled_obj)

def test_generic_extensions_final(self):
typing_extensions = pytest.importorskip('typing_extensions')

def check_final_equal(obj1, obj2):
assert obj1.__type__ == obj2.__type__
assert type(obj1) == type(obj2) == typing_extensions._FinalMeta
final_objs = [typing_extensions.Final, typing_extensions.Final[int]]

for obj in objs:
for obj in final_objs:
depickled_obj = pickle_depickle(obj, protocol=self.protocol)
assert depickled_obj == obj
if sys.version_info[:3] >= (3, 5, 3):
assert depickled_obj == obj
else:
# __eq__ does not work for Final objects in early Python 3.5
check_final_equal(obj, depickled_obj)
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved

def test_class_annotations(self):
class C:
Expand Down Expand Up @@ -2182,21 +2216,26 @@ def _all_types_to_test():
class C(typing.Generic[T]):
pass

return [
types_to_test = [
C, C[int],
T, typing.Any, typing.NoReturn, typing.Optional,
typing.Generic, typing.Union, typing.ClassVar,
T, typing.Any, typing.Optional,
typing.Generic, typing.Union,
typing.Optional[int],
typing.Generic[T],
typing.Callable[[int], typing.Any],
typing.Callable[..., typing.Any],
typing.Callable[[], typing.Any],
typing.Tuple[int, ...],
typing.Tuple[int, C[int]],
typing.ClassVar[C[int]],
typing.List[int],
typing.Dict[int, str],
]
if sys.version_info[:3] >= (3, 5, 3):
types_to_test.append(typing.ClassVar)
types_to_test.append(typing.ClassVar[C[int]])
if sys.version_info >= (3, 5, 4):
types_to_test.append(typing.NoReturn)
return types_to_test


if __name__ == '__main__':
Expand Down