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

bpo-32873: Treat type variables and special typing forms as immutable by copy and pickle #6216

Merged
merged 5 commits into from Mar 26, 2018

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Mar 24, 2018

This also fixes python/typing#512
This also fixes python/typing#511

As was discussed in both issues, some typing forms deserve to be treated as immutable by copy and pickle modules, so that:

copy(X) is X
deepcopy(X) is X
loads(dumps(X)) is X  # pickled by reference

This PR adds such behaviour to

  • Type variables
  • Special forms like Union, Any, ClassVar
  • Unsubscripted generic aliases to containers like List, Mapping, Iterable

This not only resolves inconsistencies mentioned in the issues, but also improves backwards compatibility with previous versions of Python (including 3.6).

Note that this requires some dances with __module__ for type variables (similar to NamedTuple)
because the class TypeVar itself is define in typing, while type variables should get module where they were defined.

https://bugs.python.org/issue32873

@ilevkivskyi
Copy link
Member Author

@gvanrossum @serhiy-storchaka The beta 3 is scheduled for tomorrow, and I would like this fix to get in, could one of you please review this PR, it is quite small, but I believe important, in particular because it should restore (to the extent possible) the pickle compatibility between typing in Python 3.6 and Python 3.7. Note that beta 3 is ABI freeze, I am not sure if pickle is considered part of it, but probably it is better if this gets in before.

Sorry for short notice!

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

If there's still time to get this in the current beta I think it's fine to merge as-is and deal with the extra stuff in the final beta.

global TP, TPB, TPV # for pickle
TP = TypeVar('TP')
TPB = TypeVar('TPB', bound=int)
TPV = TypeVar('TPV', bytes, str)
Copy link
Member

Choose a reason for hiding this comment

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

I don't see TPB and TPV mentioned in the for-loop below?

@@ -558,6 +565,7 @@ def __init__(self, name, *constraints, bound=None,
self.__bound__ = _type_check(bound, "Bound must be a type.")
else:
self.__bound__ = None
self._def_mod = sys._getframe(1).f_globals['__name__'] # for pickling
Copy link
Member

Choose a reason for hiding this comment

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

If the type variable is defined in some non-global scope (even inside a class) this won't work. I think that's fine (we need to support those type variables but they don't need to be picklable) but I wonder if this deserves at least a mention in the docstring.

I also notice that for the same reason a type variable defined in some non-global scope cannot be copy()'s, but it can be deepcopy()'d. That seems a little weird.

@ned-deily
Copy link
Member

(The announced cutoff for 3.7.0b3 is in about 18 hours so there's still time.)

@ilevkivskyi
Copy link
Member Author

@gvanrossum @ned-deily OK, great! I will fix this in 2-3 hours.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Looks good now!

@ilevkivskyi ilevkivskyi merged commit 8349403 into python:master Mar 26, 2018
@miss-islington
Copy link
Contributor

Thanks @ilevkivskyi for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-6264 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 26, 2018
… by copy and pickle (pythonGH-6216)

This also fixes python/typingGH-512
This also fixes python/typingGH-511

As was discussed in both issues, some typing forms deserve to be treated
as immutable by copy and pickle modules, so that:
* copy(X) is X
* deepcopy(X) is X
* loads(dumps(X)) is X  GH- pickled by reference

This PR adds such behaviour to:
* Type variables
* Special forms like Union, Any, ClassVar
* Unsubscripted generic aliases to containers like List, Mapping, Iterable

This not only resolves inconsistencies mentioned in the issues, but also
improves backwards compatibility with previous versions of Python
(including 3.6).

Note that this requires some dances with __module__ for type variables
(similar to NamedTuple) because the class TypeVar itself is define in typing,
while type variables should get module where they were defined.

https://bugs.python.org/issue32873
(cherry picked from commit 8349403)

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
miss-islington added a commit that referenced this pull request Mar 26, 2018
… by copy and pickle (GH-6216)

This also fixes python/typingGH-512
This also fixes python/typingGH-511

As was discussed in both issues, some typing forms deserve to be treated
as immutable by copy and pickle modules, so that:
* copy(X) is X
* deepcopy(X) is X
* loads(dumps(X)) is X  GH- pickled by reference

This PR adds such behaviour to:
* Type variables
* Special forms like Union, Any, ClassVar
* Unsubscripted generic aliases to containers like List, Mapping, Iterable

This not only resolves inconsistencies mentioned in the issues, but also
improves backwards compatibility with previous versions of Python
(including 3.6).

Note that this requires some dances with __module__ for type variables
(similar to NamedTuple) because the class TypeVar itself is define in typing,
while type variables should get module where they were defined.

https://bugs.python.org/issue32873
(cherry picked from commit 8349403)

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeVar equality broken? Can't pickle generic types
6 participants