Issue47143
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2022-03-28 13:51 by vstinner, last changed 2022-04-11 14:59 by admin.
Messages (20) | |||
---|---|---|---|
msg416168 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 13:51 | |
Class decorarators of attrs and stdlib dataclasses modules have to copy a class to *add* slots: * old fixed attrs issue: https://github.com/python-attrs/attrs/issues/102 * attrs issue with Python 3.11: https://github.com/python-attrs/attrs/issues/907 * dataclasses issues with slots=True: https://bugs.python.org/issue46404 In the common case, copying a class is trivial: cls2 = type(cls)(cls.__name__, cls.__bases__, cls.__dict__) Full dummy example just to change a class name without touching the original class (create a copy with a different name): --- class MyClass: def hello(self): print("Hello", self.__class__) def copy_class(cls, new_name): cls_dict = cls.__dict__.copy() # hack the dict to modify the class copy return type(cls)(new_name, cls.__bases__, cls_dict) MyClass2 = copy_class(MyClass, "MyClass2") MyClass2().hello() --- Output: --- Hello <class '__main__.MyClass2'> --- The problem is when a class uses a closure ("__class__" here): --- class MyClass: def who_am_i(self): cls = __class__ print(cls) if cls is not self.__class__: raise Exception(f"closure lies: __class__={cls} {self.__class__=}") def copy_class(cls, new_name): cls_dict = cls.__dict__.copy() # hack the dict to modify the class copy return type(cls)(new_name, cls.__bases__, cls_dict) MyClass().who_am_i() MyClass2 = copy_class(MyClass, "MyClass2") MyClass2().who_am_i() --- Output: --- <class '__main__.MyClass'> <class '__main__.MyClass'> Traceback (most recent call last): ... Exception: closure lies: __class__=<class '__main__.MyClass'> self.__class__=<class '__main__.MyClass2'> --- The attrs project uses the following complicated code to workaround this issue (to "update closures"): --- # The following is a fix for # <https://github.com/python-attrs/attrs/issues/102>. On Python 3, # if a method mentions `__class__` or uses the no-arg super(), the # compiler will bake a reference to the class in the method itself # as `method.__closure__`. Since we replace the class with a # clone, we rewrite these references so it keeps working. for item in cls.__dict__.values(): if isinstance(item, (classmethod, staticmethod)): # Class- and staticmethods hide their functions inside. # These might need to be rewritten as well. closure_cells = getattr(item.__func__, "__closure__", None) elif isinstance(item, property): # Workaround for property `super()` shortcut (PY3-only). # There is no universal way for other descriptors. closure_cells = getattr(item.fget, "__closure__", None) else: closure_cells = getattr(item, "__closure__", None) if not closure_cells: # Catch None or the empty list. continue for cell in closure_cells: try: match = cell.cell_contents is self._cls except ValueError: # ValueError: Cell is empty pass else: if match: set_closure_cell(cell, cls) --- source: https://github.com/python-attrs/attrs/blob/5c040f30e3e4b3c9c0f27c8ac6ff13d604c1818c/src/attr/_make.py#L886 The implementation of the set_closure_cell() function is really complicate since cells were mutable before Python 3.10: https://github.com/python-attrs/attrs/blob/5c040f30e3e4b3c9c0f27c8ac6ff13d604c1818c/src/attr/_compat.py#L203-L305 I propose to add a new functools.copy_class() function which copy a class and update the closures: end of the _create_slots_class() function: --- cls = type(self._cls)(...) for item in cls.__dict__.values(): ... # update closures return cls --- The alternative is not to add a function to copy a class, just only to "update closures", but IMO such API would be more error prone. I would like to implement this function, but first I would like to dicuss if it makes sense to add such function and check if it's the right abstraction. |
|||
msg416175 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 15:24 | |
It seems like the copy module doesn't support copying a class. copy.deepcopy(cls) doesn't copy a class but returns its argument, the class unchanged. |
|||
msg416176 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 15:41 | |
The pickle module doesn't copy a type but gets it from its module. The Python implementation is pickle._Pickler.save_type() which calls pickle._Pickler.save_global(). The cloudpickle module doesn't copy types neither: same behavior than pickle. Example: --- import pickle import pickletools class A: pass data = pickle.dumps(A) pickletools.dis(data) --- Output: --- 0: \x80 PROTO 4 2: \x95 FRAME 18 11: \x8c SHORT_BINUNICODE '__main__' 21: \x94 MEMOIZE (as 0) 22: \x8c SHORT_BINUNICODE 'A' 25: \x94 MEMOIZE (as 1) 26: \x93 STACK_GLOBAL 27: \x94 MEMOIZE (as 2) 28: . STOP highest protocol among opcodes = 4 --- In short, it's implemented as: getattr(__import__('__main__'), 'A') |
|||
msg416177 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 15:56 | |
pickle.dump(x) checks if x is a type since commit f048a8f6d79173cc1da1bf12c60ae06fea36762c (March 2002) of bpo-494904: Pickler.save(): Fix for SF bug #494904: Cannot pickle a class with a metaclass, reported by Dan Parisien. + if issubclass(t, TypeType): + self.save_global(object) + return Followed by a minor fix: commit 85ee491b3af3e1c124522249a52443b4d8c34c88 of bpo-502085: Don't die when issubclass(t, TypeType) fails. - if issubclass(t, TypeType): + try: + issc = issubclass(t, TypeType) + except TypeError: # t is not a class + issc = 0 copy.deepcopy(x) returns x if it's a type since commit 11ade1ddc053dcec884e2431b55fb1c1727c65d7 (June 2002) of bpo-560794. SF patch 560794 (Greg Chapman): deepcopy can't handle custom metaclasses. try: - copier = x.__deepcopy__ - except AttributeError: + issc = issubclass(type(x), type) + except TypeError: + issc = 0 + if issc: + y = _deepcopy_dispatch[type](x, memo) + else: (...) |
|||
msg416178 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 15:58 | |
More recent copy.copy() change: commit 5c1c3b4f197c57952760be37d77d73669284a607 of bpo-11480: Issue #11480: Fixed copy.copy to work with classes with custom metaclasses. + try: + issc = issubclass(cls, type) + except TypeError: # cls is not a class + issc = False + if issc: + # treat it as a regular class: + return _copy_immutable(x) |
|||
msg416179 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:02 | |
If I understand correctly, a cell content can be modified since Python 3.7: since commit 64505a1f6c0af4574e17e823b27ffe24eca44df5 of bpo-30486: bpo-30486: Allow setting cell value (#1840) Antoine Pitrou created bpo-30486 for cloudpickle: "There are use cases for setting a cell value. One such use case is for (un)pickling recursive closures (see heroic workaround here: https://github.com/cloudpipe/cloudpickle/pull/90/files#diff-d2a3618afedd4e124c532151eedbae09R74 ). Other use cases may include tinkering around and general education value." |
|||
msg416180 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:07 | |
In the Python C API, PEP 384 added PyType_FromSpec(). There is also PyStructSequence_NewType(). PEP 3121 proposed PyType_Copy() but it was never implemented: see bpo-3760. But in C, closures are implemented using a module state, or previously using a global or static variable: cell objects are not used for types implemented in C. |
|||
msg416183 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:23 | |
The same problem exists at the function level: see bpo-39805: "Copying functions doesn't actually copy them". For example, copy.deepcopy(func) returns func unchanged if it's a function. Example: --- import copy def make_closure(): closure = [] def append(value): closure.append(value) return append, closure func, closure = make_closure() func(1) func2 = copy.deepcopy(func) func2(2) print(func2 is func) print(closure) --- Output: --- True [1, 2] --- |
|||
msg416184 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:25 | |
> * old fixed attrs issue: https://github.com/python-attrs/attrs/issues/102 > * attrs issue with Python 3.11: https://github.com/python-attrs/attrs/issues/907 > * dataclasses issues with slots=True: https://bugs.python.org/issue46404 Similar bug without attrs nor dataclasses: bpo-29944 "Argumentless super() fails in classes constructed with type()". |
|||
msg416186 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:25 | |
See also the types.new_class() function: https://docs.python.org/dev/library/types.html#types.new_class Oh, I didn't know this function! |
|||
msg416189 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:29 | |
> If I understand correctly, a cell content can be modified since Python 3.7: since commit 64505a1f6c0af4574e17e823b27ffe24eca44df5 of bpo-30486 Moreover, it's possible to create a cell object since Python 3.8, commit df8d2cde63c865446468351f8f648e1c7bd45109 of bpo-35911. |
|||
msg416190 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:34 | |
bpo-32176 "Zero argument super is broken in 3.6 for methods with a hacked __class__ cell" added test_code.test_closure_injection() and fixed the CO_NOFREE flag in the code object constructor (types.CodeType). |
|||
msg416191 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 16:35 | |
> The same problem exists at the function level: see bpo-39805: "Copying functions doesn't actually copy them". See also bpo-14369 "make function __closure__ writable". |
|||
msg416221 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 22:15 | |
Note: Implementing a metaclass in Python is hard, it's easy to mess up with closures: see bpo-29270 "ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments". type.__new__() is called twice on the same type dict, and the second call overrides the __classcell__ cell value. |
|||
msg416227 - (view) | Author: Jelle Zijlstra (JelleZijlstra) * ![]() |
Date: 2022-03-28 23:17 | |
I believe the attrs code wouldn't work if a method is decorated with a decorator that wraps the original function, such as @functools.cache. |
|||
msg416230 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 23:24 | |
The stdlib types module looks like a better place for such new function, rather than the functools module. The types module documentation starts with: "This module defines utility functions to assist in dynamic creation of new types." https://docs.python.org/3/library/types.html |
|||
msg416231 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-03-28 23:32 | |
Jelle Zijlstra: > I believe the attrs code wouldn't work if a method is decorated with a decorator that wraps the original function, such as @functools.cache. What do you mean by "wouldn't work"? Do you mean that the semantics of "copy_class()" should be better defined? Currently, copy.deepcopy(MyClass) doesn't copy the class at all, it returns the class unmodified :-) @functools.cache is designed for unbound methods. Example: --- import attr import functools @attr.s(slots=True) class A: @staticmethod @functools.cache def incr(x): return x + 1 @staticmethod @functools.lru_cache def incr_lru(x): return x + 1 obj = A() print(obj.incr(1)) print(obj.incr_lru(2)) --- Output (current Python main branch, attrs 21.4.0): --- 2 3 --- @attr.s(slots=True) copies a class but then drops the original class. It doesn't create two classes which share methods, functools caches, etc. |
|||
msg416232 - (view) | Author: Jelle Zijlstra (JelleZijlstra) * ![]() |
Date: 2022-03-28 23:41 | |
I mean that the code sample above from attrs doesn't properly update the closure for wrapped methods, such as those created by @functools.cache, or any other arbitrary decorator that creates a wrapper function. Example (with Python 3.9.4 and attrs 21.4.0): % cat attrslots.py import attr import functools class Base: @classmethod def f(cls): return 3 @attr.s(slots=True) class Child(Base): x: int @classmethod @functools.cache def f(cls): return super().f() + 1 print(Child.f()) % python attrslots.py Traceback (most recent call last): File "/Users/jelle/py/pyanalyze/samples/attrslots.py", line 21, in <module> print(Child.f()) File "/Users/jelle/py/pyanalyze/samples/attrslots.py", line 18, in f return super().f() + 1 TypeError: super(type, obj): obj must be an instance or subtype of type If we provide a `types.copy_class()`, it should handle this case correctly. |
|||
msg416544 - (view) | Author: Lumír Balhar (frenzy) * | Date: 2022-04-02 10:57 | |
Do you think it's a good idea to start a PR with a copy of the implementation from attrs for Python 3.11? We can then add tests for the new function and also some for dataclasses where this new function is needed and try to find all corner cases. |
|||
msg416549 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2022-04-02 12:08 | |
Lumír Balhar: > Do you think it's a good idea to start a PR with a copy of the implementation from attrs for Python 3.11? We can then add tests for the new function and also some for dataclasses where this new function is needed and try to find all corner cases. I'm worried that attrs license is MIT with an "advertisement clause" (the MIT license must be mentioned), whereas Python has a different license. I'm planning to contact the 3 authors of the code to ask their permission. Also, I expect more feedback on this "idea" first: > I would like to implement this function, but first I would like to discuss if it makes sense to add such function and check if it's the right abstraction. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:59:57 | admin | set | github: 91299 |
2022-04-02 12:08:35 | vstinner | set | messages: + msg416549 |
2022-04-02 10:57:52 | frenzy | set | nosy:
+ frenzy messages: + msg416544 |
2022-03-28 23:41:10 | JelleZijlstra | set | messages: + msg416232 |
2022-03-28 23:32:38 | vstinner | set | messages: + msg416231 |
2022-03-28 23:24:47 | vstinner | set | messages:
+ msg416230 title: Add functools.copy_class() which updates closures -> Add types.copy_class() which updates closures |
2022-03-28 23:17:50 | JelleZijlstra | set | nosy:
+ JelleZijlstra messages: + msg416227 |
2022-03-28 22:15:23 | vstinner | set | messages: + msg416221 |
2022-03-28 16:35:36 | vstinner | set | messages: + msg416191 |
2022-03-28 16:34:19 | vstinner | set | messages: + msg416190 |
2022-03-28 16:29:09 | vstinner | set | nosy:
+ pitrou messages: + msg416189 |
2022-03-28 16:25:53 | vstinner | set | messages: + msg416186 |
2022-03-28 16:25:04 | vstinner | set | messages: + msg416184 |
2022-03-28 16:23:29 | vstinner | set | messages: + msg416183 |
2022-03-28 16:07:40 | vstinner | set | messages: + msg416180 |
2022-03-28 16:02:15 | vstinner | set | messages: + msg416179 |
2022-03-28 15:58:01 | vstinner | set | messages: + msg416178 |
2022-03-28 15:56:30 | vstinner | set | messages: + msg416177 |
2022-03-28 15:41:35 | vstinner | set | messages: + msg416176 |
2022-03-28 15:24:37 | vstinner | set | messages: + msg416175 |
2022-03-28 13:59:50 | corona10 | set | nosy:
+ corona10 |
2022-03-28 13:51:37 | vstinner | create |