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.

classification
Title: Add types.copy_class() which updates closures
Type: Stage:
Components: Library (Lib) Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: JelleZijlstra, corona10, eric.smith, frenzy, petr.viktorin, pitrou, rhettinger, serhiy.storchaka, vstinner
Priority: normal Keywords:

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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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:57adminsetgithub: 91299
2022-04-02 12:08:35vstinnersetmessages: + msg416549
2022-04-02 10:57:52frenzysetnosy: + frenzy
messages: + msg416544
2022-03-28 23:41:10JelleZijlstrasetmessages: + msg416232
2022-03-28 23:32:38vstinnersetmessages: + msg416231
2022-03-28 23:24:47vstinnersetmessages: + msg416230
title: Add functools.copy_class() which updates closures -> Add types.copy_class() which updates closures
2022-03-28 23:17:50JelleZijlstrasetnosy: + JelleZijlstra
messages: + msg416227
2022-03-28 22:15:23vstinnersetmessages: + msg416221
2022-03-28 16:35:36vstinnersetmessages: + msg416191
2022-03-28 16:34:19vstinnersetmessages: + msg416190
2022-03-28 16:29:09vstinnersetnosy: + pitrou
messages: + msg416189
2022-03-28 16:25:53vstinnersetmessages: + msg416186
2022-03-28 16:25:04vstinnersetmessages: + msg416184
2022-03-28 16:23:29vstinnersetmessages: + msg416183
2022-03-28 16:07:40vstinnersetmessages: + msg416180
2022-03-28 16:02:15vstinnersetmessages: + msg416179
2022-03-28 15:58:01vstinnersetmessages: + msg416178
2022-03-28 15:56:30vstinnersetmessages: + msg416177
2022-03-28 15:41:35vstinnersetmessages: + msg416176
2022-03-28 15:24:37vstinnersetmessages: + msg416175
2022-03-28 13:59:50corona10setnosy: + corona10
2022-03-28 13:51:37vstinnercreate