From 3c300a04a831605ce468c54fbbaa12bce578be1c Mon Sep 17 00:00:00 2001 From: Lumir Balhar Date: Wed, 15 May 2024 10:43:50 +0200 Subject: [PATCH 01/12] Fix test_extract_class_dict for Python 3.13 beta 1 Resolves: https://github.com/cloudpipe/cloudpickle/issues/533 --- tests/cloudpickle_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 5aa4baca..af5ef3e3 100644 --- a/tests/cloudpickle_test.py +++ b/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() From 4f9a8230460f641506ef9eae5f4e80a3e6f14434 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 17:44:27 +0200 Subject: [PATCH 02/12] Upgrade to actions/setup-python@v5 and add 3.13 to the list --- .github/workflows/testing.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 6a56b47e..1abca4ea 100644 --- a/.github/workflows/testing.yml +++ b/.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 From 103f9ec142d6da017d388df14ff592de10323ced Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 17:45:10 +0200 Subject: [PATCH 03/12] Add 313 to tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 5985d534..a9d3eded 100644 --- a/tox.ini +++ b/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 From 16e6652d90f8004cf2f90db5348ef3283351b0ef Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 18:10:34 +0200 Subject: [PATCH 04/12] Fix typo in comment --- tests/cloudpickle_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index c7daaef2..cb8ad08b 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2072,7 +2072,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" @@ -2083,7 +2083,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__""" From d6696c26a66f7882f9f02289cb5acf592a7e85ad Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 19:00:17 +0200 Subject: [PATCH 05/12] For un-interning of __qualname__ in _function_getstate --- cloudpickle/cloudpickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 88f9b12a..b9108613 100644 --- a/cloudpickle/cloudpickle.py +++ b/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__, From 5279785fd8ce27af1df797e16320b2c34cb647a7 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 19:01:07 +0200 Subject: [PATCH 06/12] Fix seamingly wrong XXX statement in a test docstring --- tests/cloudpickle_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index cb8ad08b..655cf4df 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -2141,8 +2141,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.""" From c8ff4a823cff81d01ea8e9a22c2706105dc559de Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Wed, 31 Jul 2024 19:04:54 +0200 Subject: [PATCH 07/12] Update changelog to also refer to this PR --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 73677a96..eaf6ca9a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,7 +3,8 @@ - Some improvements to make cloudpickle more deterministic when pickling dynamic functions and classes. - ([PR #524](https://github.com/cloudpipe/cloudpickle/pull/524)) + ([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`. From ac8c5c6a178deda0253f5ae09206254aa6a12dc6 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Fri, 11 Oct 2024 14:52:54 +0200 Subject: [PATCH 08/12] Trigger CI again From 25d54932fd26c7f674e0c9fc13b90147f7e86c20 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 11 Oct 2024 17:15:35 +0200 Subject: [PATCH 09/12] Add test reproducing cloudpickle droppping `__firstlineno__` --- tests/cloudpickle_test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 655cf4df..1e594784 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -336,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): From c2f0312a1605243f0090e10225c947ef27d64573 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 11 Oct 2024 17:17:12 +0200 Subject: [PATCH 10/12] ensure `cloudpickle` restores `__firstlineno__` in classes --- cloudpickle/cloudpickle.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index b9108613..e8da3c7e 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -1167,6 +1167,13 @@ 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__". + obj.__firstlineno__ = state["__firstlineno__"] + if registry is not None: for subclass in registry: obj.register(subclass) From eba58488e055bfcf77b0ea916241624fe913af74 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Fri, 11 Oct 2024 17:42:16 +0200 Subject: [PATCH 11/12] More specific entry in CHANGES.md --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index eaf6ca9a..cd17ec37 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,7 +2,7 @@ ====================== - Some improvements to make cloudpickle more deterministic when pickling - dynamic functions and classes. + 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)) From 4ad41f697ad5945690f915cc5465dabc8faa50ed Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 11 Oct 2024 16:43:06 +0100 Subject: [PATCH 12/12] Update cloudpickle/cloudpickle.py Co-authored-by: Olivier Grisel --- cloudpickle/cloudpickle.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index e8da3c7e..8c50ba17 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -1171,7 +1171,11 @@ def _class_setstate(obj, state): 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__". + # 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: