-
Notifications
You must be signed in to change notification settings - Fork 184
Pickling of generic annotations/types in 3.5+ #318
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
Changes from all commits
a3299ed
73ab6df
bc0c368
55489f4
ef4a263
333d6b0
26e06f5
eb1cebd
63bc551
f889731
236b339
554a4c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1787,9 +1787,6 @@ def g(): | |
|
|
||
| self.assertEqual(f2.__doc__, f.__doc__) | ||
|
|
||
| @unittest.skipIf(sys.version_info < (3, 7), | ||
| "Pickling type annotations isn't supported for py36 and " | ||
| "below.") | ||
| def test_wraps_preserves_function_annotations(self): | ||
| def f(x): | ||
| pass | ||
|
|
@@ -1804,79 +1801,7 @@ 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_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 | ||
|
|
||
| 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 | ||
|
|
||
| with pytest.raises(Exception): | ||
| cloudpickle.dumps(f.__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 | ||
|
|
||
| # Pickling and unpickling must be done in different processes when | ||
| # testing dynamic classes (see #313) | ||
|
|
||
| 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 | ||
|
|
||
| try: | ||
| cloudpickle.dumps(A.__annotations__) | ||
| except Exception: | ||
| pass | ||
| else: | ||
| raise AssertionError | ||
|
|
||
| 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__") | ||
|
|
||
| @unittest.skipIf(sys.version_info < (3, 7), | ||
| "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) | ||
| t = typing.Union[list, int] | ||
| assert pickle_depickle(t) == t | ||
|
|
||
|
|
@@ -2142,16 +2067,17 @@ 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_type(self): | ||
| 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] | ||
|
|
||
| # Identity is not part of the typing contract: only test for | ||
| # equality instead. | ||
| assert pickle_depickle(C[int], protocol=self.protocol) == C[int] | ||
|
|
||
| with subprocess_worker(protocol=self.protocol) as worker: | ||
|
|
||
|
|
@@ -2170,33 +2096,55 @@ def check_generic(generic, origin, type_value): | |
| 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): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, If this syntax causes a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but it seemed worth it to do this change because then it tests pickling/unpickling class attributes on 3.5 too. That could happen in real code if a pickle created from 3.6+ is loaded. Another small reason was that assigning
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
FYI, neither
I agree. But the current syntax makes this test somewhat obscure, for future maintenance I would prefer using the |
||
| with subprocess_worker(protocol=self.protocol) as worker: | ||
| for type_ in _all_types_to_test(): | ||
| # 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"] | ||
| MyClass.__annotations__ = {'attribute': 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 | ||
| assert obj.__annotations__["attribute"] == expected_type | ||
| assert obj.method.__annotations__["arg"] == expected_type | ||
| assert ( | ||
| obj.method.__annotations__["return"] == expected_type | ||
| ) | ||
| return "ok" | ||
|
|
||
| obj = MyClass() | ||
| assert check_annotations(obj, type_) == "ok" | ||
| assert worker.run(check_annotations, obj, type_) == "ok" | ||
|
|
||
| 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: | ||
| depickled_obj = pickle_depickle(obj, protocol=self.protocol) | ||
| assert depickled_obj == obj | ||
|
|
||
| def test_class_annotations(self): | ||
| class C: | ||
| pass | ||
| C.__annotations__ = {'a': int} | ||
|
|
||
| C1 = pickle_depickle(C, protocol=self.protocol) | ||
| assert C1.__annotations__ == C.__annotations__ | ||
|
|
||
| def test_function_annotations(self): | ||
| def f(a: int) -> str: | ||
| pass | ||
|
|
||
| f1 = pickle_depickle(f, protocol=self.protocol) | ||
| assert f1.__annotations__ == f.__annotations__ | ||
|
|
||
|
|
||
| class Protocol2CloudPickleTest(CloudPickleTest): | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be enabled if
typing_extensionsis not available, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
save_parametrized_type_hintsis useful to pickle:typing_extensionsconstruct such asLiteralandFinaltypingconstruct, such asClassVar,Union,Tuple,Generic...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But do want to enable a half broken feature in cloudpickle when
typing_extensionsis not installed?I would rather have the type annotations to be dropped or raise an explicit error message that states that installing
typing_extensionson Python 3.6 an earlier.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this code breaks if
typing_extensionsis not installed: for instance, the CI tests the pickling oftypingconstructs in all entries of the CI whethertyping_extensionsis installed or not (typing_extensionsis installed in only one entry in the CI).All
typing_extensionsobjects in this branch are created through this process, which include import guards.Can you precise what the "half-broken feature" is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typing extensions is only an extension of the
typingmodule inPython 3.5-3.6. It only exposes new type annotations and does not impact the behavior ofcloudpicklewhen pickling type annotations from thetypingmodule.