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

classmethod tests fail with Python 3.13 (Python reverted to pre-3.9 behavior) #259

Closed
hroncok opened this issue Mar 7, 2024 · 2 comments · May be fixed by #260
Closed

classmethod tests fail with Python 3.13 (Python reverted to pre-3.9 behavior) #259

hroncok opened this issue Mar 7, 2024 · 2 comments · May be fixed by #260

Comments

@hroncok
Copy link
Contributor

hroncok commented Mar 7, 2024

Similarily to #160 the tests now fail with Python 3.13.0a4.

To reproduce, I did:

$ git clone [email protected]:GrahamDumpleton/wrapt.git
$ cd wrapt/
[wrapt (develop)]$ tox -e py313
py313: failed with unable to determine pip install command: attempting to parse '' into a command failed
  py313: FAIL code 1 (0.20 seconds)
  evaluation failed :( (0.25 seconds)

I did not understand the error, so I changed:

diff --git a/setup.cfg b/setup.cfg
index 9c282c9..9c391b3 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -91,8 +91,6 @@ python =
 deps =
   coverage
   pytest
-install_command =
-  py311,py311-{without,install,disable}-extensions: python -m pip install --no-binary coverage {opts} {packages}
 commands =
   python -m coverage run --rcfile {toxinidir}/setup.cfg -m pytest -v {posargs} {toxinidir}/tests
 setenv =

And continued:

[wrapt (develop *)]$ tox -e py313
.pkg: _optional_hooks> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_wheel> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
py313: install_package> python -I -m pip install --force-reinstall --no-deps .../wrapt/.tox/.tmp/package/2/wrapt-1.16.0.tar.gz
py313: commands[0]> python -m coverage run --rcfile .../wrapt/setup.cfg -m pytest -v .../wrapt/tests
============================= test session starts ==============================
platform linux -- Python 3.13.0a4, pytest-8.0.2, pluggy-1.4.0 -- .../wrapt/.tox/py313/bin/python
cachedir: .tox/py313/.pytest_cache
rootdir: .../wrapt
configfile: setup.cfg
collecting ... collected 438 items

...

=================================== FAILURES ===================================
_____________ TestCallingOuterClassMethod.test_class_call_function _____________

self = <test_outer_classmethod.TestCallingOuterClassMethod testMethod=test_class_call_function>

    def test_class_call_function(self):
        # Test calling classmethod. Prior to Python 3.9, the instance
        # and class passed to the wrapper will both be None because our
        # decorator is surrounded by the classmethod decorator. The
        # classmethod decorator doesn't bind the method and treats it
        # like a normal function, explicitly passing the class as the
        # first argument with the actual arguments following that. This
        # was only finally fixed in Python 3.9. For more details see:
        # https://bugs.python.org/issue19072
    
        _args = (1, 2)
        _kwargs = {'one': 1, 'two': 2}
    
        @wrapt.decorator
        def _decorator(wrapped, instance, args, kwargs):
            if PYXY < (3, 9):
                self.assertEqual(instance, None)
                self.assertEqual(args, (Class,)+_args)
            else:
                self.assertEqual(instance, Class)
                self.assertEqual(args, _args)
    
            self.assertEqual(kwargs, _kwargs)
            self.assertEqual(wrapped.__module__, _function.__module__)
            self.assertEqual(wrapped.__name__, _function.__name__)
    
            return wrapped(*args, **kwargs)
    
        @_decorator
        def _function(*args, **kwargs):
            return args, kwargs
    
        class Class(object):
            @classmethod
            @_decorator
            def _function(cls, *args, **kwargs):
                return (args, kwargs)
    
>       result = Class._function(*_args, **_kwargs)

tests/test_outer_classmethod.py:160: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_outer_classmethod.py:141: in _decorator
    self.assertEqual(instance, Class)
E   AssertionError: None != <class 'test_outer_classmethod.TestCallin[54 chars]ass'>
___________ TestCallingOuterClassMethod.test_instance_call_function ____________

self = <test_outer_classmethod.TestCallingOuterClassMethod testMethod=test_instance_call_function>

    def test_instance_call_function(self):
        # Test calling classmethod via class instance. Prior to Python
        # 3.9, the instance and class passed to the wrapper will both be
        # None because our decorator is surrounded by the classmethod
        # decorator. The classmethod decorator doesn't bind the method
        # and treats it like a normal function, explicitly passing the
        # class as the first argument with the actual arguments
        # following that. This was only finally fixed in Python 3.9. For
        # more details see: https://bugs.python.org/issue19072
    
        _args = (1, 2)
        _kwargs = {'one': 1, 'two': 2}
    
        @wrapt.decorator
        def _decorator(wrapped, instance, args, kwargs):
            if PYXY < (3, 9):
                self.assertEqual(instance, None)
                self.assertEqual(args, (Class,)+_args)
            else:
                self.assertEqual(instance, Class)
                self.assertEqual(args, _args)
    
            self.assertEqual(kwargs, _kwargs)
            self.assertEqual(wrapped.__module__, _function.__module__)
            self.assertEqual(wrapped.__name__, _function.__name__)
    
            return wrapped(*args, **kwargs)
    
        @_decorator
        def _function(*args, **kwargs):
            return args, kwargs
    
        class Class(object):
            @classmethod
            @_decorator
            def _function(cls, *args, **kwargs):
                return (args, kwargs)
    
>       result = Class()._function(*_args, **_kwargs)

tests/test_outer_classmethod.py:202: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_outer_classmethod.py:183: in _decorator
    self.assertEqual(instance, Class)
E   AssertionError: None != <class 'test_outer_classmethod.TestCallin[57 chars]ass'>
_____________ TestSynchronized.test_synchronized_outer_classmethod _____________

self = <test_synchronized_lock.TestSynchronized testMethod=test_synchronized_outer_classmethod>

    def test_synchronized_outer_classmethod(self):
        # Prior to Python 3.9 this isn't detected as a class method
        # call, as the classmethod decorator doesn't bind the wrapped
        # function to the class before calling and just calls it direct,
        # explicitly passing the class as first argument. For more
        # details see: https://bugs.python.org/issue19072
    
        if PYXY < (3, 9):
            _lock0 = getattr(C4.function2, '_synchronized_lock', None)
        else:
            _lock0 = getattr(C4, '_synchronized_lock', None)
        self.assertEqual(_lock0, None)
    
        c4.function2()
    
        if PYXY < (3, 9):
            _lock1 = getattr(C4.function2, '_synchronized_lock', None)
        else:
            _lock1 = getattr(C4, '_synchronized_lock', None)
>       self.assertNotEqual(_lock1, None)
E       AssertionError: None == None

tests/test_synchronized_lock.py:181: AssertionError
----------------------------- Captured stdout call -----------------------------
function2
=========================== short test summary info ============================
FAILED tests/test_outer_classmethod.py::TestCallingOuterClassMethod::test_class_call_function
FAILED tests/test_outer_classmethod.py::TestCallingOuterClassMethod::test_instance_call_function
FAILED tests/test_synchronized_lock.py::TestSynchronized::test_synchronized_outer_classmethod
======================== 3 failed, 435 passed in 0.83s =========================
py313: exit 1 (1.12 seconds) .../wrapt> python -m coverage run --rcfile .../wrapt/setup.cfg -m pytest -v .../wrapt/tests pid=1586485
.pkg: _exit> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  py313: FAIL code 1 (3.27=setup[2.15]+cmd[1.12] seconds)
  evaluation failed :( (3.33 seconds)

When searching the issues here, I found #160 which looks very similar, if not the same.

So I searched https://docs.python.org/3.13/whatsnew/3.13.html for "classmethod" and found:

Removed chained classmethod descriptors (introduced in gh-63272). This can no longer be used to wrap other descriptors such as property. The core design of this feature was flawed and caused a number of downstream problems. To "pass-through" a classmethod, consider using the __wrapped__ attribute that was added in Python 3.10. (Contributed by Raymond Hettinger in gh-89519.)

@hroncok
Copy link
Contributor Author

hroncok commented Mar 7, 2024

I'm currently looking at a073c97 + 945a980 and I will attempt to change the ifs to cover the reintroduced old behavior.

hroncok added a commit to hroncok/wrapt that referenced this issue Mar 7, 2024
Fixes GrahamDumpleton#259

The failures were:

    =================================== FAILURES ===================================
    _____________ TestCallingOuterClassMethod.test_class_call_function _____________

    self = <test_outer_classmethod.TestCallingOuterClassMethod testMethod=test_class_call_function>

        def test_class_call_function(self):
            # Test calling classmethod. Prior to Python 3.9, the instance
            # and class passed to the wrapper will both be None because our
            # decorator is surrounded by the classmethod decorator. The
            # classmethod decorator doesn't bind the method and treats it
            # like a normal function, explicitly passing the class as the
            # first argument with the actual arguments following that. This
            # was only finally fixed in Python 3.9. For more details see:
            # https://bugs.python.org/issue19072

            _args = (1, 2)
            _kwargs = {'one': 1, 'two': 2}

            @wrapt.decorator
            def _decorator(wrapped, instance, args, kwargs):
                if PYXY < (3, 9):
                    self.assertEqual(instance, None)
                    self.assertEqual(args, (Class,)+_args)
                else:
                    self.assertEqual(instance, Class)
                    self.assertEqual(args, _args)

                self.assertEqual(kwargs, _kwargs)
                self.assertEqual(wrapped.__module__, _function.__module__)
                self.assertEqual(wrapped.__name__, _function.__name__)

                return wrapped(*args, **kwargs)

            @_decorator
            def _function(*args, **kwargs):
                return args, kwargs

            class Class(object):
                @classmethod
                @_decorator
                def _function(cls, *args, **kwargs):
                    return (args, kwargs)

    >       result = Class._function(*_args, **_kwargs)

    tests/test_outer_classmethod.py:160:
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
    tests/test_outer_classmethod.py:141: in _decorator
        self.assertEqual(instance, Class)
    E   AssertionError: None != <class 'test_outer_classmethod.TestCallin[54 chars]ass'>
    ___________ TestCallingOuterClassMethod.test_instance_call_function ____________

    self = <test_outer_classmethod.TestCallingOuterClassMethod testMethod=test_instance_call_function>

        def test_instance_call_function(self):
            # Test calling classmethod via class instance. Prior to Python
            # 3.9, the instance and class passed to the wrapper will both be
            # None because our decorator is surrounded by the classmethod
            # decorator. The classmethod decorator doesn't bind the method
            # and treats it like a normal function, explicitly passing the
            # class as the first argument with the actual arguments
            # following that. This was only finally fixed in Python 3.9. For
            # more details see: https://bugs.python.org/issue19072

            _args = (1, 2)
            _kwargs = {'one': 1, 'two': 2}

            @wrapt.decorator
            def _decorator(wrapped, instance, args, kwargs):
                if PYXY < (3, 9):
                    self.assertEqual(instance, None)
                    self.assertEqual(args, (Class,)+_args)
                else:
                    self.assertEqual(instance, Class)
                    self.assertEqual(args, _args)

                self.assertEqual(kwargs, _kwargs)
                self.assertEqual(wrapped.__module__, _function.__module__)
                self.assertEqual(wrapped.__name__, _function.__name__)

                return wrapped(*args, **kwargs)

            @_decorator
            def _function(*args, **kwargs):
                return args, kwargs

            class Class(object):
                @classmethod
                @_decorator
                def _function(cls, *args, **kwargs):
                    return (args, kwargs)

    >       result = Class()._function(*_args, **_kwargs)

    tests/test_outer_classmethod.py:202:
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
    tests/test_outer_classmethod.py:183: in _decorator
        self.assertEqual(instance, Class)
    E   AssertionError: None != <class 'test_outer_classmethod.TestCallin[57 chars]ass'>
    _____________ TestSynchronized.test_synchronized_outer_classmethod _____________

    self = <test_synchronized_lock.TestSynchronized testMethod=test_synchronized_outer_classmethod>

        def test_synchronized_outer_classmethod(self):
            # Prior to Python 3.9 this isn't detected as a class method
            # call, as the classmethod decorator doesn't bind the wrapped
            # function to the class before calling and just calls it direct,
            # explicitly passing the class as first argument. For more
            # details see: https://bugs.python.org/issue19072

            if PYXY < (3, 9):
                _lock0 = getattr(C4.function2, '_synchronized_lock', None)
            else:
                _lock0 = getattr(C4, '_synchronized_lock', None)
            self.assertEqual(_lock0, None)

            c4.function2()

            if PYXY < (3, 9):
                _lock1 = getattr(C4.function2, '_synchronized_lock', None)
            else:
                _lock1 = getattr(C4, '_synchronized_lock', None)
    >       self.assertNotEqual(_lock1, None)
    E       AssertionError: None == None

    tests/test_synchronized_lock.py:181: AssertionError
    ----------------------------- Captured stdout call -----------------------------
    function2
    =========================== short test summary info ============================
    FAILED tests/test_outer_classmethod.py::TestCallingOuterClassMethod::test_class_call_function
    FAILED tests/test_outer_classmethod.py::TestCallingOuterClassMethod::test_instance_call_function
    FAILED tests/test_synchronized_lock.py::TestSynchronized::test_synchronized_outer_classmethod
    ======================== 3 failed, 435 passed in 0.83s =========================

To fix the same failures on Python 3.9,
they were adjusted in the past. For details see
GrahamDumpleton#160

However, Python 3.13 reverted the change from 3.9,
so this adds an upper bound for the conditionals.

To make the conditionals easier to read, the if-else branches were switched.
@hroncok
Copy link
Contributor Author

hroncok commented Mar 7, 2024

I've opened #260

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant