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

Fix test_extract_class_dict for Python 3.13 beta 1 #534

Merged
merged 13 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

- Some improvements to make cloudpickle more deterministic when pickling
dynamic functions and classes.
ogrisel marked this conversation as resolved.
Show resolved Hide resolved
([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`.
Expand Down
4 changes: 2 additions & 2 deletions cloudpickle/cloudpickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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__),
Copy link
Contributor

Choose a reason for hiding this comment

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

It was previously not interned but now it is without rebuilding the string?
Not super clear why this behavior changed, I could not find any lead in the python3.13 release note.

But if it does the trick, this seems fine like this, this should not be too costly to do.

"__annotations__": func.__annotations__,
"__kwdefaults__": func.__kwdefaults__,
"__defaults__": func.__defaults__,
Expand Down
13 changes: 8 additions & 5 deletions tests/cloudpickle_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -2067,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"
Expand All @@ -2078,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__"""
Expand Down Expand Up @@ -2136,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."""

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading