From dafaae30bca0647c14d26f9f739384280e749294 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 19 Feb 2020 14:44:10 +0100 Subject: [PATCH 1/4] don't pickle the annotations of a dynamic class --- cloudpickle/cloudpickle.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 893a47fba..68bd20df8 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -581,6 +581,11 @@ def save_dynamic_class(self, obj): if isinstance(__dict__, property): type_kwargs['__dict__'] = __dict__ + if sys.version_info < (3, 7): + # Although annotations were added in Python 3.4, It is not possible + # to properly pickle them until Python 3.7. (See #193) + clsdict.pop('__annotations__', None) + save = self.save write = self.write From 2416bb88ab4a607f6203c971ecff569707e9b507 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 19 Feb 2020 14:44:30 +0100 Subject: [PATCH 2/4] test pickling a dynamic class with annotations --- tests/cloudpickle_test.py | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index f717dcc1f..de6ab733f 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -1802,6 +1802,47 @@ def g(x): self.assertEqual(f2.__annotations__, f.__annotations__) + @unittest.skipIf(sys.version_info >= (3, 7), + "pickling annotations is supported starting Python 3.7") + def test_annotations_silent_dropping(self): + # Because of limitations of typing module, cloudpickle does not pickle + # the type annotations of a dynamic function or class for Python < 3.7 + + class UnpicklableAnnotation: + # Mock Annotation metaclass that errors out loudly if we try to + # pickle one of its instances + def __reduce__(self): + raise Exception("not picklable") + + unpickleable_annotation = UnpicklableAnnotation() + + def f(a: unpickleable_annotation): + return a + + class A: + a: unpickleable_annotation + + with pytest.raises(Exception): + cloudpickle.dumps(f.__annotations__) + + with pytest.raises(Exception): + cloudpickle.dumps(A.__annotations__) + + f_pickle_string = cloudpickle.dumps(f, protocol=self.protocol) + A_pickle_string = cloudpickle.dumps(A, protocol=self.protocol) + + code = """if 1: + import pickle + + depickled_f = pickle.loads({s1}) + assert depickled_f.__annotations__ == {{}} + + depickled_a = pickle.loads({s2}) + assert not hasattr(depickled_a, "__annotations__") + """.format(s1=f_pickle_string, s2=A_pickle_string, + protocol=self.protocol) + assert_run_python_script(code) + @unittest.skipIf(sys.version_info < (3, 7), """This syntax won't work on py2 and pickling annotations isn't supported for py37 and below.""") From df5a5ebc63d1f9ac978f121b77af1742429b5fc6 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 19 Feb 2020 15:24:04 +0100 Subject: [PATCH 3/4] pickle and depickle dynamic class in != processes --- CHANGES.md | 4 +++ tests/cloudpickle_test.py | 58 ++++++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 914e5e66c..fdec87447 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,10 @@ **This version requires Python 3.5 or later** +- Stop pickling the annotations of a dynamic class for Python < 3.6 + (follow up on #276) + ([issue #347](https://github.com/cloudpipe/cloudpickle/issues/347)) + 1.3.0 ===== diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index de6ab733f..3af6bfb38 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -1804,7 +1804,7 @@ def g(x): @unittest.skipIf(sys.version_info >= (3, 7), "pickling annotations is supported starting Python 3.7") - def test_annotations_silent_dropping(self): + def test_function_annotations_silent_dropping(self): # Because of limitations of typing module, cloudpickle does not pickle # the type annotations of a dynamic function or class for Python < 3.7 @@ -1819,29 +1819,55 @@ def __reduce__(self): def f(a: unpickleable_annotation): return a - class A: - a: unpickleable_annotation - with pytest.raises(Exception): cloudpickle.dumps(f.__annotations__) - with pytest.raises(Exception): - cloudpickle.dumps(A.__annotations__) + depickled_f = pickle_depickle(f, protocol=self.protocol) + assert depickled_f.__annotations__ == {} + + @unittest.skipIf(sys.version_info >= (3, 7) or sys.version_info < (3, 6), + "pickling annotations is supported starting Python 3.7") + def test_class_annotations_silent_dropping(self): + # Because of limitations of typing module, cloudpickle does not pickle + # the type annotations of a dynamic function or class for Python < 3.7 - f_pickle_string = cloudpickle.dumps(f, protocol=self.protocol) - A_pickle_string = cloudpickle.dumps(A, protocol=self.protocol) + # Pickling and unpickling must be done in different processes when + # testing dynamic classes (see #313) - code = """if 1: - import pickle + code = '''if 1: + import cloudpickle + import sys + + class UnpicklableAnnotation: + # Mock Annotation metaclass that errors out loudly if we try to + # pickle one of its instances + def __reduce__(self): + raise Exception("not picklable") + + unpickleable_annotation = UnpicklableAnnotation() + + class A: + a: unpickleable_annotation - depickled_f = pickle.loads({s1}) - assert depickled_f.__annotations__ == {{}} + try: + cloudpickle.dumps(A.__annotations__) + except Exception: + pass + else: + raise AssertionError - depickled_a = pickle.loads({s2}) + sys.stdout.buffer.write(cloudpickle.dumps(A, protocol={protocol})) + ''' + cmd = [sys.executable, '-c', code.format(protocol=self.protocol)] + proc = subprocess.Popen( + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE + ) + proc.wait() + out, err = proc.communicate() + assert proc.returncode == 0, err + + depickled_a = pickle.loads(out) assert not hasattr(depickled_a, "__annotations__") - """.format(s1=f_pickle_string, s2=A_pickle_string, - protocol=self.protocol) - assert_run_python_script(code) @unittest.skipIf(sys.version_info < (3, 7), """This syntax won't work on py2 and pickling annotations From e5e4c81d6c1ef90805a0bc0ef327256cb9592e17 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Thu, 20 Feb 2020 15:22:06 +0100 Subject: [PATCH 4/4] Fix skip messages + move imports --- tests/cloudpickle_test.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 3af6bfb38..d64d2001d 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -22,6 +22,8 @@ import weakref import os import enum +import typing +from functools import wraps import pytest @@ -1784,11 +1786,9 @@ def g(): self.assertEqual(f2.__doc__, f.__doc__) @unittest.skipIf(sys.version_info < (3, 7), - "This syntax won't work on py2 and pickling annotations " - "isn't supported for py37 and below.") + "Pickling type annotations isn't supported for py36 and " + "below.") def test_wraps_preserves_function_annotations(self): - from functools import wraps - def f(x): pass @@ -1870,12 +1870,11 @@ class A: assert not hasattr(depickled_a, "__annotations__") @unittest.skipIf(sys.version_info < (3, 7), - """This syntax won't work on py2 and pickling annotations - isn't supported for py37 and below.""") + "Pickling type hints isn't supported for py36" + " and below.") def test_type_hint(self): # Try to pickle compound typing constructs. This would typically fail # on Python < 3.7 (See #193) - import typing t = typing.Union[list, int] assert pickle_depickle(t) == t