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.

Sign up for GitHub

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

Jump to bottom

Free-threding (no-gil) support #269

Merged
merged 4 commits into from
Sep 20, 2024
Merged

Free-threding (no-gil) support #269

merged 4 commits into from
Sep 20, 2024

Conversation

FFY00
Copy link
Contributor

@FFY00 FFY00 commented Sep 19, 2024

hroncok and others added 3 commits September 20, 2024 00:37
@hroncok @FFY00
Fix classmethod tests with Python 3.13+
9a38c9f 
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.

Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
tox: fix install_command for non py311 environments
cd88299 
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
tox: add Python 3.13 and 3.14 environments
226eddc 
Signed-off-by: Filipe Laíns <lains@riseup.net>
@GrahamDumpleton
Copy link
Owner

Python 3.6 and 3.7 should be dropped from the test matrix as GitHub actions now longer supports using them when testing.

Classifiers in setup.cfg should similarly also drop those versions, and add 3.13 (not 3.14).

Thanks for submitting the updates. Was hoping to get to doing it this coming week, so this saves me a bit of work.

@GrahamDumpleton
Copy link
Owner

BTW, not sure what the free threading support is going to be, so can we just get the minimal changes to ensure Python 3.13 is okay first and merge that.

@FFY00
Copy link
Contributor Author

FFY00 commented Sep 20, 2024

Sounds good. Can you merge #260? Then I will open a PR with adding 3.13 to tox and CI.

BTW, not sure what the free threading support is going to be, so can we just get the minimal changes to ensure Python 3.13 is okay first and merge that.

Yep, that was my plan too. See what changes are needed for free-threading support, implement them, and then let you figure out what kind of support to provide based on that work.

@GrahamDumpleton
Copy link
Owner

I made the 3.6/3.7 changes first so can ensure that GitHub action based tests pass.

@GrahamDumpleton
Copy link
Owner

Damn. Need to modify the GitHub actions workflow file. I was making changes through GitHub web site rather than doing an actual check out. Don't know how you can add a file not part of PR through GitHub web site to modify. So need to do a checkout to fix, or merge and fix after. 😔

I will need to do it a bit later after finish something else first.

@GrahamDumpleton GrahamDumpleton marked this pull request as ready for review September 20, 2024 00:39
@GrahamDumpleton GrahamDumpleton merged commit 448ca19 into GrahamDumpleton:develop Sep 20, 2024
23 of 33 checks passed
@GrahamDumpleton
Copy link
Owner

You can start trying to build off develop again. Was merged and have been trying to fix up GitHub actions build/test. Getting close to have it working.

BTW, not that it will matter to you, for next wrapt version will be bumping version up to 1.17.0 since dropping 3.6/3.7 support, plus also going to be adding some changes which prefer not be in patch level update.

@FFY00 FFY00 mentioned this pull request Sep 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@FFY00 @GrahamDumpleton @hroncok