Fix test_extract_class_dict for Python 3.13 beta 1

[frenzymadness]

Resolves: #533

[ogrisel]

Thanks for the PR. Ideally we would need to update the CI. Do you know if that can be easily achieved with GitHub Actions?

[frenzymadness]

Python 3.13 is already available in setup-python action, see: https://github.com/actions/python-versions/blob/main/versions-manifest.json

So it should be easy to add 3.13 to the CI here.

[ogrisel]

The remaining failures seems to be all related to @tomMoral's tests for deterministic pickling implemented in #524.

I had a quick look but it's not easy to understand why cloudpickle with Python 3.13 no longer leads to deterministic pickling.

[ogrisel]

More specifically, for test_deterministic_dynamic_class_attr_ordering_for_chained_pickling, the first difference is the lack of memoization of the "CloudPickleTest.test_deterministic_dynamic_class_attr_ordering_for_chained_pickling.<locals>.A.__init__" string, which looks like the fully qualified name (__qualname__ attribute) of the __init__ method of the class being pickled in the test.

https://github.com/cloudpipe/cloudpickle/actions/runs/10183782743/job/28169551899?pr=534#step:6:168

[ogrisel]

Maybe this is the cause: https://github.com/cloudpipe/cloudpickle/pull/524/files#r1399015881

[ogrisel]

Ok d6696c2 did the trick! I will update the changelog and wait some time to let @tomMoral review this change.

[QuLogic]

Python 3.13 final release is in a week, and this appears to be passing tests; is there any thing remaining to complete this (other than review, I suppose)?

[tomMoral]

LGTM! Thanks @frenzymadness and @ogrisel for the debug of this part.

Note that python 3.13 now have a sys.isinterned which might be useful to better investigate this issue.

[tomMoral]

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.

[ogrisel]

After triggering the tests again with CPython 3.13.0 we get new failures on macOS. Will investigate.

[ogrisel]

The __firstlineno__ is present in the first pickle string but missing from the second in:

            class A:
                """Simple class definition"""

                pass

            A_dump = w.run(cloudpickle.dumps, A)
            check_deterministic_pickle(A_dump, cloudpickle.dumps(A))

[pierreglaser]

I did a bit more digging. In Python 3.13, setting the __module__ attribute of some class has the side effect of deleting its __firstlineno__ attribute:

In [1]: class A:
      ...:     pass
      ...:

In [2]: A.__firstlineno__
Out[2]: 1

In [3]: A.__module__ = "anymodule"

In [4]: A.__firstlineno__
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[4], line 1
----> 1 A.__firstlineno__

AttributeError: type object 'A' has no attribute '__firstlineno__'

Note that this snippet does not involve cloudpickle.

The cloudpickle reproducibility tests fail because of the folllowing sequence of events:

• a class is created, and pickled using cloudpickle, generating a first pickle string. Since it's __module__ is left untouched in the process, the class has a __firstlineno__ attribute, which is pickled as part of the class's state by cloudpickle
• at unpickling time, cloudpickle first restores the class's __firstlineno__ attribute, and then __module__, thus deleting the restored __firstlineno__ attribute.
• this depickled class is then repickled another time, yielding a second pickle string, which differs from the first one as it not contain a __firstlineno__ attribute in it. Consequently, the test, which asserts the equality of the two strings, fails.

There is a question to be had about whether the __firstlineno__ attribute of a depickled dynamically created class has any valuable meaning. But from a user perspective, the least surprising behaviour that cloudpickle can have is probably to restore this argument as it was as unpickling time, which is what I propose we do.

[pierreglaser]

Merging, thanks @tomMoral @frenzymadness @ogrisel!