From 7b4bb6dc4c3df4be8c1f7d0fcb968bf872bea7ce Mon Sep 17 00:00:00 2001 From: valtron Date: Tue, 10 Mar 2020 19:39:28 -0600 Subject: [PATCH 01/11] Add support for pickling dynamic generics on 3.7+ --- cloudpickle/cloudpickle.py | 20 ++++++++++++-- cloudpickle/cloudpickle_fast.py | 4 +-- tests/cloudpickle_test.py | 47 +++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 5cde86cfa..84f5f3812 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -644,8 +644,9 @@ def save_dynamic_class(self, obj): else: # "Regular" class definition: tp = type(obj) + bases = _get_bases(obj) self.save_reduce(_make_skeleton_class, - (tp, obj.__name__, obj.__bases__, type_kwargs, + (tp, obj.__name__, bases, type_kwargs, _ensure_tracking(obj), None), obj=obj) @@ -1163,10 +1164,17 @@ class id will also reuse this class definition. The "extra" variable is meant to be a dict (or None) that can be used for forward compatibility shall the need arise. """ - skeleton_class = type_constructor(name, bases, type_kwargs) + skeleton_class = _make_new_class(type_constructor, name, bases, type_kwargs) return _lookup_class_or_track(class_tracker_id, skeleton_class) +def _make_new_class(type_constructor, name, bases, type_kwargs): + return types.new_class( + name, bases, {'metaclass': type_constructor}, + lambda ns: ns.update(type_kwargs) + ) + + def _rehydrate_skeleton_class(skeleton_class, class_dict): """Put attributes from `class_dict` back on `skeleton_class`. @@ -1268,3 +1276,11 @@ def _typevar_reduce(obj): if module_and_name is None: return (_make_typevar, _decompose_typevar(obj)) return (getattr, module_and_name) + + +def _get_bases(typ): + if hasattr(typ, '__orig_bases__'): + bases_attr = '__orig_bases__' + else: + bases_attr = '__bases__' + return getattr(typ, bases_attr) diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index 47e70de94..49453d5b1 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -28,7 +28,7 @@ _is_dynamic, _extract_code_globals, _BUILTIN_TYPE_NAMES, DEFAULT_PROTOCOL, _find_imported_submodules, _get_cell_contents, _is_importable_by_name, _builtin_type, Enum, _ensure_tracking, _make_skeleton_class, _make_skeleton_enum, - _extract_class_dict, dynamic_subimport, subimport, _typevar_reduce, + _extract_class_dict, dynamic_subimport, subimport, _typevar_reduce, _get_bases, ) load, loads = _pickle.load, _pickle.loads @@ -76,7 +76,7 @@ def _class_getnewargs(obj): if isinstance(__dict__, property): type_kwargs['__dict__'] = __dict__ - return (type(obj), obj.__name__, obj.__bases__, type_kwargs, + return (type(obj), obj.__name__, _get_bases(obj), type_kwargs, _ensure_tracking(obj), None) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index aa64f5740..58a43493c 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2130,6 +2130,53 @@ def test_pickle_importable_typevar(self): from typing import AnyStr assert AnyStr is pickle_depickle(AnyStr, protocol=self.protocol) + @unittest.skipIf(sys.version_info < (3, 7), + "Pickling generics not supported below py37") + def test_generic(self): + from typing import ( + Optional, TypeVar, Generic, Tuple, Callable, + Dict, Any, ClassVar, NoReturn, Union, List, + ) + + T = TypeVar('T') + + class C(Generic[T]): + pass + + objs = [ + C, C[int], + T, Any, NoReturn, Optional, Generic, + Union, ClassVar, + Optional[int], + Generic[T], + Callable[[int], Any], + Callable[..., Any], + Callable[[], Any], + Tuple[int, ...], + Tuple[int, C[int]], + ClassVar[C[int]], + List[int], + Dict[int, str], + ] + + for obj in objs: + _ = pickle_depickle(obj, protocol=self.protocol) + + @unittest.skipIf(sys.version_info < (3, 7), + "Pickling generics not supported below py37") + def test_generic_extensions(self): + typing_extensions = pytest.importorskip('typing_extensions') + + objs = [ + typing_extensions.Literal, + typing_extensions.Final, + typing_extensions.Literal['a'], + typing_extensions.Final[int], + ] + + for obj in objs: + _ = pickle_depickle(obj, protocol=self.protocol) + class Protocol2CloudPickleTest(CloudPickleTest): From 7fd4412f18b0fedb6b82883bed76a7a076057cfb Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sat, 14 Mar 2020 19:43:59 +0100 Subject: [PATCH 02/11] Add new test for class with type hints in subprocess --- tests/cloudpickle_test.py | 47 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 58a43493c..2881d1df8 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -50,6 +50,7 @@ from .testutils import subprocess_pickle_echo from .testutils import assert_run_python_script +from .testutils import subprocess_worker _TEST_GLOBAL_VARIABLE = "default_value" @@ -2162,6 +2163,52 @@ class C(Generic[T]): for obj in objs: _ = pickle_depickle(obj, protocol=self.protocol) + @unittest.skipIf(sys.version_info < (3, 7), + "Pickling type hints not supported below py37") + def test_locally_defined_class_with_type_hints(self): + from typing import ( + Optional, TypeVar, Generic, Tuple, Callable, + Dict, Any, ClassVar, NoReturn, Union, List, + ) + + T = TypeVar('T') + + class C(Generic[T]): + pass + + all_types = [ + C, C[int], + T, Any, NoReturn, Optional, Generic, + Union, ClassVar, + Optional[int], + Generic[T], + Callable[[int], Any], + Callable[..., Any], + Callable[[], Any], + Tuple[int, ...], + Tuple[int, C[int]], + ClassVar[C[int]], + List[int], + Dict[int, str], + ] + + with subprocess_worker(protocol=self.protocol) as worker: + for type_ in all_types: + class MyClass: + attribute: type_ + + def method(self, arg: type_) -> type_: + return arg + + def check_annotations(obj, expected_type): + assert obj.__annotations__["attribute"] is expected_type + assert obj.method.__annotations__["arg"] is expected_type + return "ok" + + obj = MyClass() + assert worker.run(check_annotations, obj, type_) == "ok" + + @unittest.skipIf(sys.version_info < (3, 7), "Pickling generics not supported below py37") def test_generic_extensions(self): From d67aad0b1d85c999820fab8057effe20ba0cea96 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sat, 14 Mar 2020 19:48:45 +0100 Subject: [PATCH 03/11] PEP8 --- tests/cloudpickle_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 2881d1df8..35962abce 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2208,7 +2208,6 @@ def check_annotations(obj, expected_type): obj = MyClass() assert worker.run(check_annotations, obj, type_) == "ok" - @unittest.skipIf(sys.version_info < (3, 7), "Pickling generics not supported below py37") def test_generic_extensions(self): From 20a8835b1fb30fc840f0316924926ce15632fb23 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sat, 14 Mar 2020 19:55:49 +0100 Subject: [PATCH 04/11] Workaround Python 3.5 syntax error --- tests/cloudpickle_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 35962abce..039e63515 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2194,11 +2194,17 @@ class C(Generic[T]): with subprocess_worker(protocol=self.protocol) as worker: for type_ in all_types: + # The type annotation syntax causes a SyntaxError on Python 3.5 + code = textwrap.dedent("""\ class MyClass: attribute: type_ def method(self, arg: type_) -> type_: return arg + """) + ns = {"type_": type_} + exec(code, ns) + MyClass = ns["MyClass"] def check_annotations(obj, expected_type): assert obj.__annotations__["attribute"] is expected_type From 59e648e458ca504edbc733a77aab629f898bcd85 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sat, 14 Mar 2020 20:20:32 +0100 Subject: [PATCH 05/11] Sanity check --- tests/cloudpickle_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 039e63515..4ec659efe 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2212,6 +2212,7 @@ def check_annotations(obj, expected_type): return "ok" obj = MyClass() + assert check_annotations(obj, type_) == "ok" assert worker.run(check_annotations, obj, type_) == "ok" @unittest.skipIf(sys.version_info < (3, 7), From dc25224351e3dc7a1543e8f6a218314ca6bae5ba Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sat, 14 Mar 2020 20:25:11 +0100 Subject: [PATCH 06/11] Simpler non-regression test for dynamic typevar memoization --- tests/cloudpickle_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 4ec659efe..864b1dddc 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2122,6 +2122,12 @@ def test_pickle_dynamic_typevar(self): for attr in attr_list: assert getattr(T, attr) == getattr(depickled_T, attr) + def test_pickle_dynamic_typevar_memoization(self): + T = typing.TypeVar('T') + depickled_T1, depickled_T2 = pickle_depickle((T, T), + protocol=self.protocol) + assert depickled_T1 is depickled_T2 + def test_pickle_importable_typevar(self): from .mypkg import T T1 = pickle_depickle(T, protocol=self.protocol) From 82b5f3151a545263ac4b04121a2a04dcee05354e Mon Sep 17 00:00:00 2001 From: valtron Date: Sat, 14 Mar 2020 14:01:02 -0600 Subject: [PATCH 07/11] Address review comments --- CHANGES.md | 3 ++ cloudpickle/cloudpickle.py | 13 +++--- tests/cloudpickle_test.py | 85 +++++++------------------------------- 3 files changed, 22 insertions(+), 79 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c4efa23dd..5fe29f376 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,9 @@ and expand the support for pickling `TypeVar` instances (dynamic or non-dynamic) to Python 3.5-3.6 ([PR #350](https://github.com/cloudpipe/cloudpickle/pull/350)) +- Add support for pickling dynamic generics on 3.7+ + ([PR #351](https://github.com/cloudpipe/cloudpickle/pull/351)) + 1.3.0 ===== diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 84f5f3812..8ad128b1f 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -644,9 +644,8 @@ def save_dynamic_class(self, obj): else: # "Regular" class definition: tp = type(obj) - bases = _get_bases(obj) self.save_reduce(_make_skeleton_class, - (tp, obj.__name__, bases, type_kwargs, + (tp, obj.__name__, _get_bases(obj), type_kwargs, _ensure_tracking(obj), None), obj=obj) @@ -1164,15 +1163,11 @@ class id will also reuse this class definition. The "extra" variable is meant to be a dict (or None) that can be used for forward compatibility shall the need arise. """ - skeleton_class = _make_new_class(type_constructor, name, bases, type_kwargs) - return _lookup_class_or_track(class_tracker_id, skeleton_class) - - -def _make_new_class(type_constructor, name, bases, type_kwargs): - return types.new_class( + skeleton_class = types.new_class( name, bases, {'metaclass': type_constructor}, lambda ns: ns.update(type_kwargs) ) + return _lookup_class_or_track(class_tracker_id, skeleton_class) def _rehydrate_skeleton_class(skeleton_class, class_dict): @@ -1280,7 +1275,9 @@ def _typevar_reduce(obj): def _get_bases(typ): if hasattr(typ, '__orig_bases__'): + # For generic types (see PEP 560) bases_attr = '__orig_bases__' else: + # For regular class objects bases_attr = '__bases__' return getattr(typ, bases_attr) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 864b1dddc..1f98094f9 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2139,67 +2139,15 @@ def test_pickle_importable_typevar(self): @unittest.skipIf(sys.version_info < (3, 7), "Pickling generics not supported below py37") - def test_generic(self): - from typing import ( - Optional, TypeVar, Generic, Tuple, Callable, - Dict, Any, ClassVar, NoReturn, Union, List, - ) - - T = TypeVar('T') - - class C(Generic[T]): - pass - - objs = [ - C, C[int], - T, Any, NoReturn, Optional, Generic, - Union, ClassVar, - Optional[int], - Generic[T], - Callable[[int], Any], - Callable[..., Any], - Callable[[], Any], - Tuple[int, ...], - Tuple[int, C[int]], - ClassVar[C[int]], - List[int], - Dict[int, str], - ] - - for obj in objs: - _ = pickle_depickle(obj, protocol=self.protocol) + def test_generic_type(self): + for obj in _generic_objects_to_test(): + pickle_depickle(obj, protocol=self.protocol) @unittest.skipIf(sys.version_info < (3, 7), "Pickling type hints not supported below py37") def test_locally_defined_class_with_type_hints(self): - from typing import ( - Optional, TypeVar, Generic, Tuple, Callable, - Dict, Any, ClassVar, NoReturn, Union, List, - ) - - T = TypeVar('T') - - class C(Generic[T]): - pass - - all_types = [ - C, C[int], - T, Any, NoReturn, Optional, Generic, - Union, ClassVar, - Optional[int], - Generic[T], - Callable[[int], Any], - Callable[..., Any], - Callable[[], Any], - Tuple[int, ...], - Tuple[int, C[int]], - ClassVar[C[int]], - List[int], - Dict[int, str], - ] - with subprocess_worker(protocol=self.protocol) as worker: - for type_ in all_types: + for type_ in _generic_objects_to_test(): # The type annotation syntax causes a SyntaxError on Python 3.5 code = textwrap.dedent("""\ class MyClass: @@ -2221,21 +2169,6 @@ def check_annotations(obj, expected_type): assert check_annotations(obj, type_) == "ok" assert worker.run(check_annotations, obj, type_) == "ok" - @unittest.skipIf(sys.version_info < (3, 7), - "Pickling generics not supported below py37") - def test_generic_extensions(self): - typing_extensions = pytest.importorskip('typing_extensions') - - objs = [ - typing_extensions.Literal, - typing_extensions.Final, - typing_extensions.Literal['a'], - typing_extensions.Final[int], - ] - - for obj in objs: - _ = pickle_depickle(obj, protocol=self.protocol) - class Protocol2CloudPickleTest(CloudPickleTest): @@ -2267,5 +2200,15 @@ def test_lookup_module_and_qualname_stdlib_typevar(): assert name == 'AnyStr' +def _generic_objects_to_test(): + T = typing.TypeVar('T') + + class C(typing.Generic[T]): + pass + + yield C + yield C[int] + + if __name__ == '__main__': unittest.main() From 5326c8c03b31dadedc8ffae516afdba0d1fb5e6f Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sun, 15 Mar 2020 10:36:57 +0100 Subject: [PATCH 08/11] Fix memoization of dynamic typevars --- cloudpickle/cloudpickle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 8ad128b1f..537a00cc5 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -446,7 +446,7 @@ def dump(self, obj): raise def save_typevar(self, obj): - self.save_reduce(*_typevar_reduce(obj)) + self.save_reduce(*_typevar_reduce(obj), obj=obj) dispatch[typing.TypeVar] = save_typevar From 43ef75359f98042019cd8826bc06d670451e5458 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sun, 15 Mar 2020 16:29:11 +0100 Subject: [PATCH 09/11] Update tests/cloudpickle_test.py --- tests/cloudpickle_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 1f98094f9..4ffbbd3ec 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2163,6 +2163,7 @@ def method(self, arg: type_) -> type_: def check_annotations(obj, expected_type): assert obj.__annotations__["attribute"] is expected_type assert obj.method.__annotations__["arg"] is expected_type + assert obj.method.__annotations__["return"] is expected_type return "ok" obj = MyClass() From 68545022ed4ac66eb9d1193eca1c8a0294b950bc Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sun, 15 Mar 2020 20:20:42 +0100 Subject: [PATCH 10/11] Update CHANGES.md Co-Authored-By: Pierre Glaser --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 5fe29f376..6fb058b8a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,8 @@ and expand the support for pickling `TypeVar` instances (dynamic or non-dynamic) to Python 3.5-3.6 ([PR #350](https://github.com/cloudpipe/cloudpickle/pull/350)) -- Add support for pickling dynamic generics on 3.7+ +- Add support for pickling dynamic classes subclassing `typing.Generic` + instances on Python 3.7+ ([PR #351](https://github.com/cloudpipe/cloudpickle/pull/351)) 1.3.0 From 5791b8416ea8760bb1b9f5d790ddee86264bfea9 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Sun, 15 Mar 2020 20:30:37 +0100 Subject: [PATCH 11/11] Test dynamic generic subclasses and annotated dynamic classes --- tests/cloudpickle_test.py | 47 ++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 4ffbbd3ec..eab45763b 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2140,14 +2140,36 @@ def test_pickle_importable_typevar(self): @unittest.skipIf(sys.version_info < (3, 7), "Pickling generics not supported below py37") def test_generic_type(self): - for obj in _generic_objects_to_test(): - pickle_depickle(obj, protocol=self.protocol) + T = typing.TypeVar('T') + + class C(typing.Generic[T]): + pass + + assert pickle_depickle(C, protocol=self.protocol) is C + assert pickle_depickle(C[int], protocol=self.protocol) is C[int] + + with subprocess_worker(protocol=self.protocol) as worker: + + def check_generic(generic, origin, type_value): + 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 + 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" @unittest.skipIf(sys.version_info < (3, 7), "Pickling type hints not supported below py37") def test_locally_defined_class_with_type_hints(self): with subprocess_worker(protocol=self.protocol) as worker: - for type_ in _generic_objects_to_test(): + for type_ in _all_types_to_test(): # The type annotation syntax causes a SyntaxError on Python 3.5 code = textwrap.dedent("""\ class MyClass: @@ -2201,14 +2223,27 @@ def test_lookup_module_and_qualname_stdlib_typevar(): assert name == 'AnyStr' -def _generic_objects_to_test(): +def _all_types_to_test(): T = typing.TypeVar('T') class C(typing.Generic[T]): pass - yield C - yield C[int] + return [ + C, C[int], + T, typing.Any, typing.NoReturn, typing.Optional, + typing.Generic, typing.Union, typing.ClassVar, + 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 __name__ == '__main__':