Accomodate class state restoration for Python 3.13 (#534)

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Pierre Glaser <pierreglaser@msn.com>

.github/workflows/testing.yml

@@ -12,7 +12,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python 3.11
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: 3.11
     - name: Install pre-commit
@@ -29,7 +29,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
-        python_version: ["3.8", "3.9", "3.10", "3.11", "3.12", "pypy-3.9"]
+        python_version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "pypy-3.9"]
         exclude:
           # Do not test all minor versions on all platforms, especially if they
           # are not the oldest/newest supported versions
@@ -50,7 +50,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python_version }}
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python_version }}
         allow-prereleases: true
@@ -91,7 +91,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python_version }}
     - name: Install project and dependencies
@@ -127,7 +127,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python_version }}
     - name: Install project and dependencies
@@ -155,7 +155,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python_version }}
     - name: Install downstream project and dependencies
@@ -180,7 +180,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python_version }}
     - name: Install project and tests dependencies

CHANGES.md

@@ -2,8 +2,9 @@
 ======================
 
 - Some improvements to make cloudpickle more deterministic when pickling
-  dynamic functions and classes.
-  ([PR #524](https://github.com/cloudpipe/cloudpickle/pull/524))
+  dynamic functions and classes, in particular with CPython 3.13.
+  ([PR #524](https://github.com/cloudpipe/cloudpickle/pull/524) and
+   [PR #534](https://github.com/cloudpipe/cloudpickle/pull/534))
 
 - Fix a problem with the joint usage of cloudpickle's `_whichmodule` and
   `multiprocessing`.

cloudpickle/cloudpickle.py

@@ -370,7 +370,7 @@ def func():
                 # sys.modules.
                 if name is not None and name.startswith(prefix):
                     # check whether the function can address the sub-module
-                    tokens = set(name[len(prefix):].split("."))
+                    tokens = set(name[len(prefix) :].split("."))
                     if not tokens - set(code.co_names):
                         subimports.append(sys.modules[name])
     return subimports
@@ -707,7 +707,7 @@ def _function_getstate(func):
         # Hack to circumvent non-predictable memoization caused by string interning.
         # See the inline comment in _class_setstate for details.
         "__name__": "".join(func.__name__),
-        "__qualname__": func.__qualname__,
+        "__qualname__": "".join(func.__qualname__),
         "__annotations__": func.__annotations__,
         "__kwdefaults__": func.__kwdefaults__,
         "__defaults__": func.__defaults__,
@@ -1167,6 +1167,17 @@ def _class_setstate(obj, state):
             # Indeed the Pickler's memoizer relies on physical object identity to break
             # cycles in the reference graph of the object being serialized.
             setattr(obj, attrname, attr)
+
+    if sys.version_info >= (3, 13) and "__firstlineno__" in state:
+        # Set the Python 3.13+ only __firstlineno__  attribute one more time, as it
+        # will be automatically deleted by the `setattr(obj, attrname, attr)` call
+        # above when `attrname` is "__firstlineno__". We assume that preserving this
+        # information might be important for some users and that it not stale in the
+        # context of cloudpickle usage, hence legitimate to propagate. Furthermore it
+        # is necessary to do so to keep deterministic chained pickling as tested in
+        # test_deterministic_str_interning_for_chained_dynamic_class_pickling.
+        obj.__firstlineno__ = state["__firstlineno__"]
+
     if registry is not None:
         for subclass in registry:
             obj.register(subclass)

tests/cloudpickle_test.py

@@ -110,7 +110,12 @@ def method_c(self):
             return "c"
 
     clsdict = _extract_class_dict(C)
-    assert list(clsdict.keys()) == ["C_CONSTANT", "__doc__", "method_c"]
+    expected_keys = ["C_CONSTANT", "__doc__", "method_c"]
+    # New attribute in Python 3.13 beta 1
+    # https://github.com/python/cpython/pull/118475
+    if sys.version_info >= (3, 13):
+        expected_keys.insert(2, "__firstlineno__")
+    assert list(clsdict.keys()) == expected_keys
     assert clsdict["C_CONSTANT"] == 43
     assert clsdict["__doc__"] is None
     assert clsdict["method_c"](C()) == C().method_c()
@@ -331,6 +336,25 @@ def g():
         g = pickle_depickle(f(), protocol=self.protocol)
         self.assertEqual(g(), 2)
 
+    def test_class_no_firstlineno_deletion_(self):
+        # `__firstlineno__` is a new attribute of classes introduced in Python 3.13.
+        # This attribute used to be automatically deleted when unpickling a class as a
+        # consequence of cloudpickle setting a class's `__module__` attribute at
+        # unpickling time (see https://github.com/python/cpython/blob/73c152b346a18ed8308e469bdd232698e6cd3a63/Objects/typeobject.c#L1353-L1356).
+        # This deletion would cause tests like
+        # `test_deterministic_dynamic_class_attr_ordering_for_chained_pickling` to fail.
+        # This test makes sure that the attribute `__firstlineno__` is preserved
+        # across a cloudpickle roundtrip.
+
+        class A:
+            pass
+
+        if hasattr(A, "__firstlineno__"):
+            A_roundtrip = pickle_depickle(A, protocol=self.protocol)
+            assert hasattr(A_roundtrip, "__firstlineno__")
+            assert A_roundtrip.__firstlineno__ == A.__firstlineno__
+
+
     def test_dynamically_generated_class_that_uses_super(self):
         class Base:
             def method(self):
@@ -2067,7 +2091,7 @@ class A:
             # If the `__doc__` attribute is defined after some other class
             # attribute, this can cause class attribute ordering changes due to
             # the way we reconstruct the class definition in
-            # `_make_class_skeleton`, which creates the class and thus its
+            # `_make_skeleton_class`, which creates the class and thus its
             # `__doc__` attribute before populating the class attributes.
             class A:
                 name = "A"
@@ -2078,7 +2102,7 @@ class A:
 
             # If a `__doc__` is defined on the `__init__` method, this can
             # cause ordering changes due to the way we reconstruct the class
-            # with `_make_class_skeleton`.
+            # with `_make_skeleton_class`.
             class A:
                 def __init__(self):
                     """Class definition with explicit __init__"""
@@ -2136,8 +2160,6 @@ def test_dynamic_class_determinist_subworker_tuple_memoization(self):
             # Arguments' tuple is memoized in the main process but not in the
             # subprocess as the tuples do not share the same id in the loaded
             # class.
-
-            # XXX - this does not seem to work, and I am not sure there is an easy fix.
             class A:
                 """Class with potential tuple memoization issues."""
 

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist = py{38, 39, 310, 311, 312, py3}
+envlist = py{38, 39, 310, 311, 312, 313, py3}
 
 [testenv]
 deps = -rdev-requirements.txt