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

Test failures with Python 3.13 #581

Closed
opoplawski opened this issue Jun 14, 2024 · 5 comments · Fixed by #587
Closed

Test failures with Python 3.13 #581

opoplawski opened this issue Jun 14, 2024 · 5 comments · Fixed by #587

Comments

@opoplawski
Copy link

Fedora rawhide has updated to Python 3.13b2. toolz tests are failing with:

____________________________ test_curried_namespace ____________________________

    def test_curried_namespace():
        exceptions = import_module('toolz.curried.exceptions')
        namespace = {}


        def curry_namespace(ns):
            return {
                name: toolz.curry(f) if should_curry(f) else f
                for name, f in ns.items() if '__' not in name
            }

        from_toolz = curry_namespace(vars(toolz))
        from_exceptions = curry_namespace(vars(exceptions))
        namespace.update(toolz.merge(from_toolz, from_exceptions))

        namespace = toolz.valfilter(callable, namespace)
        curried_namespace = toolz.valfilter(callable, toolz.curried.__dict__)

        if namespace != curried_namespace:
            missing = set(namespace) - set(curried_namespace)
            if missing:
                raise AssertionError('There are missing functions in toolz.curried:\n    %s'
                                     % '    \n'.join(sorted(missing)))
            extra = set(curried_namespace) - set(namespace)
            if extra:
                raise AssertionError('There are extra functions in toolz.curried:\n    %s'
                                     % '    \n'.join(sorted(extra)))
            unequal = toolz.merge_with(list, namespace, curried_namespace)
            unequal = toolz.valfilter(lambda x: x[0] != x[1], unequal)
            messages = []
            for name, (orig_func, auto_func) in sorted(unequal.items()):
                if name in from_exceptions:
                    messages.append('%s should come from toolz.curried.exceptions' % name)
                elif should_curry(getattr(toolz, name)):
                    messages.append('%s should be curried from toolz' % name)
                else:
                    messages.append('%s should come from toolz and NOT be curried' % name)
>           raise AssertionError('\n'.join(messages))
E           AssertionError: map should come from toolz and NOT be curried

toolz/tests/test_curried.py:117: AssertionError
_________________________________ test_excepts _________________________________

    def test_excepts():
        # These are descriptors, make sure this works correctly.
        assert excepts.__name__ == 'excepts'
>       assert (
            'A wrapper around a function to catch exceptions and\n'
            '    dispatch to a handler.\n'
        ) in excepts.__doc__
E       AssertionError: assert 'A wrapper around a function to catch exceptions and\n    dispatch to a handler.\n' in 'A wrapper around a function to catch exceptions and\ndispatch to a handler.\n\nThis is like a functional try/except block, in the same way that\nifexprs are functional if/else blocks.\n\nExamples\n--------\n>>> excepting = excepts(\n...     ValueError,\n...     lambda a: [1, 2].index(a),\n...     lambda _: -1,\n... )\n>>> excepting(1)\n0\n>>> excepting(3)\n-1\n\nMultiple exceptions and default except clause.\n\n>>> excepting = excepts((IndexError, KeyError), lambda a: a[0])\n>>> excepting([])\n>>> excepting([1])\n1\n>>> excepting({})\n>>> excepting({0: 1})\n1\n'
E        +  where 'A wrapper around a function to catch exceptions and\ndispatch to a handler.\n\nThis is like a functional try/except block, in the same way that\nifexprs are functional if/else blocks.\n\nExamples\n--------\n>>> excepting = excepts(\n...     ValueError,\n...     lambda a: [1, 2].index(a),\n...     lambda _: -1,\n... )\n>>> excepting(1)\n0\n>>> excepting(3)\n-1\n\nMultiple exceptions and default except clause.\n\n>>> excepting = excepts((IndexError, KeyError), lambda a: a[0])\n>>> excepting([])\n>>> excepting([1])\n1\n>>> excepting({})\n>>> excepting({0: 1})\n1\n' = excepts.__doc__

toolz/tests/test_functoolz.py:741: AssertionError
____________________________ test_num_required_args ____________________________

    def test_num_required_args():
        assert num_required_args(lambda: None) == 0
        assert num_required_args(lambda x: None) == 1
        assert num_required_args(lambda x, *args: None) == 1
        assert num_required_args(lambda x, **kwargs: None) == 1
        assert num_required_args(lambda x, y, *args, **kwargs: None) == 2
>       assert num_required_args(map) == 2
E       assert 1 == 2
E        +  where 1 = num_required_args(map)

toolz/tests/test_inspect_args.py:276: AssertionError
________________________ test_inspect_wrapped_property _________________________

    def test_inspect_wrapped_property():
        class Wrapped(object):
            def __init__(self, func):
                self.func = func

            def __call__(self, *args, **kwargs):
                return self.func(*args, **kwargs)

            @property
            def __wrapped__(self):
                return self.func

        func = lambda x: x
        wrapped = Wrapped(func)
        assert inspect.signature(func) == inspect.signature(wrapped)

>       assert num_required_args(Wrapped) is None
E       AssertionError: assert 1 is None
E        +  where 1 = num_required_args(<class 'test_inspect_args.test_inspect_wrapped_property.<locals>.Wrapped'>)

toolz/tests/test_inspect_args.py:485: AssertionError

The test_except error is likely related to:

https://docs.python.org/3.13/whatsnew/3.13.html#other-language-changes

"""
Compiler now strip indents from docstrings. This will reduce the size of bytecode cache (e.g. .pyc file). For example, cache file size for sqlalchemy.orm.session in SQLAlchemy 2.0 is reduced by about 5%. This change will affect tools using docstrings, like doctest.
"""

python/cpython#81283

The spaces before "dispatch" are stripped.

@AdamWill
Copy link
Contributor

Fixing the test_except error is trivial, I have a commit for that already, but working on the others before sending a PR. The test_curried_namespace and test_num_required_args failures are ultimately caused by a CPython bug - sent a fix as python/cpython#120528 . Will look at the test_inspect_wrapped_property failure now.

@AdamWill
Copy link
Contributor

AdamWill commented Jun 14, 2024

The test_inspect_wrapped_property failure doesn't seem to be Python 3.13 specific; it's also failing in Fedora 39 and Fedora 40 builds, which use Python 3.12.3. The assertion it makes seems odd, actually:

def test_inspect_wrapped_property():
    class Wrapped(object):
        def __init__(self, func):
            self.func = func

        def __call__(self, *args, **kwargs):
            return self.func(*args, **kwargs)

        @property
        def __wrapped__(self):
            return self.func

    func = lambda x: x
    wrapped = Wrapped(func)
    assert inspect.signature(func) == inspect.signature(wrapped)

    assert num_required_args(Wrapped) is None

why should num_required_args(Wrapped) be None? Surely the correct answer is 1, which is the answer we're getting now? If you recreate the test in an interpreter, you can't do Wrapped(), it errors out:

>>> Wrapped()
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    Wrapped()
    ~~~~~~~^^
TypeError: Wrapped.__init__() missing 1 required positional argument: 'func'

so...my working theory is this changed between Python 3.12.1 (the test passed in this build, which used that version of Python) and 3.12.3, but it's not really a bug. If anything it was broken before. Will dig into it a bit more and decide on a sensible resolution.

edit: Aha, yeah, so this was python/cpython#112006 , fixed in Python 3.11.9 and Python 3.12.3. With an older Python, unwrap(Wrapped) gives a property object, and inspect.signature(Wrapped) raises a ValueError. That sends us down the backup path in _check_sigspec which tries to get the info out of _signatures.py, but of course we don't have any info for a made-up wrapper there, so _num_required_args just returns None, and that's what we're asserting in this test.

But with fixed Python, unwrap(Wrapped) gives Wrapped itself, and inspect.signature(Wrapped) works, so we get the right answer.

I'll patch the test to expect the appropriate answer for the version of Python it's running on, I guess.

@AdamWill
Copy link
Contributor

#582 fixes the issues aside from the one caused by python/cpython#120528 , we should not work around that here I don't think, just wait for cpython to be fixed.

@jrbourbeau
Copy link
Member

I ran the current dev branch against the latest Python 3.13 release candidate over in https://github.com/jrbourbeau/toolz/commits/test-313/ (here's the CI build https://github.com/jrbourbeau/toolz/actions/runs/11076655508/job/30780207947). It looks like tests are passing today. The build still fails due to some coverage constraint, but that's a separate point.

@AdamWill is this safe to close, or am I missing something? We can also add a Python 3.13 build to CI

@AdamWill
Copy link
Contributor

yup, I believe so. The cpython fix went in a while back and should be in the RC. Adding 3.13 to CI would be great. thanks!

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.

3 participants