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_completion_param_annotations fails in Python 3.13 #2002

Closed
carlwgeorge opened this issue Jun 21, 2024 · 4 comments Β· Fixed by #2003
Closed

test_completion_param_annotations fails in Python 3.13 #2002

carlwgeorge opened this issue Jun 21, 2024 · 4 comments Β· Fixed by #2003
Assignees

Comments

@carlwgeorge
Copy link
Contributor

Fedora has switched to Python 3.13 in its Rawhide branch in preparation for Fedora 41. This has brought to our attention that jedi's test_completion_param_annotations test fails with this version of Python.

🐍 .py3.13 ❯ pytest -k test_completion_param_annotations
========================================== test session starts ===========================================
platform linux -- Python 3.13.0b2, pytest-6.2.5, py-1.11.0, pluggy-1.5.0
rootdir: /home/carl/development/jedi, configfile: pytest.ini, testpaths: jedi, test
collected 3888 items / 3887 deselected / 1 selected                                                      

test/test_api/test_interpreter.py F                                                                [100%]

================================================ FAILURES ================================================
___________________________________ test_completion_param_annotations ____________________________________

    def test_completion_param_annotations():
        # Need to define this function not directly in Python. Otherwise Jedi is too
        # clever and uses the Python code instead of the signature object.
        code = 'def foo(a: 1, b: str, c: int = 1.0) -> bytes: pass'
        exec(code, locals())
        script = jedi.Interpreter('foo', [locals()])
>       c, = script.complete()
E       ValueError: not enough values to unpack (expected 1, got 0)

test/test_api/test_interpreter.py:315: ValueError
======================================== short test summary info =========================================
FAILED test/test_api/test_interpreter.py::test_completion_param_annotations - ValueError: not enough va...
=================================== 1 failed, 3887 deselected in 0.55s ===================================
@PeterJCLaw PeterJCLaw self-assigned this Jun 23, 2024
PeterJCLaw added a commit to PeterJCLaw/jedi that referenced this issue Jun 23, 2024
In Python 3.13 the `locals` function now returns a fresh mapping
each time it's called (when called in a function). We thus need
to store a reference to the mapping being used, rather than
re-fetching it each time.

Since we don't actually need to modify the locals within the scope
of the test function itself, it suffices to use our own mapping
here rather than the result of calling `locals`, which fully
isolates this test from the nature of that function.

Fixes davidhalter#2002
PeterJCLaw added a commit to PeterJCLaw/jedi that referenced this issue Jul 1, 2024
In Python 3.13 the `locals` function now returns a fresh mapping
each time it's called (when called in a function). We thus need
to store a reference to the mapping being used, rather than
re-fetching it each time.

Since we don't actually need to modify the locals within the scope
of the test function itself, it suffices to use our own mapping
here rather than the result of calling `locals`, which fully
isolates this test from the nature of that function.

Fixes davidhalter#2002
PeterJCLaw added a commit to PeterJCLaw/jedi that referenced this issue Jul 1, 2024
In Python 3.13 the `locals` function now returns a fresh mapping
each time it's called (when called in a function). We thus need
to store a reference to the mapping being used, rather than
re-fetching it each time.

Since we don't actually need to modify the locals within the scope
of the test function itself, it suffices to use our own mapping
here rather than the result of calling `locals`, which fully
isolates this test from the nature of that function.

Fixes davidhalter#2002
@PeterJCLaw
Copy link
Collaborator

@carlwgeorge thanks for reporting this. I've been working on Python 3.13 support and while the majority of the updates have been in tests, there have been some source changes to ensure correct behaviour.

I wanted to let you know that we're working on this and while I'm hopeful of getting support ready in the next few weeks I can't commit to a timeline. I appreciate from the link you shared to the Fedora wiki that that might be tight for your own timelines, so thought it best to let you know where we stand at the moment.

@carlwgeorge
Copy link
Contributor Author

Thanks for the update. No worries about the timeline, it's quite common that things don't line up ideally between projects. I certainly don't want you to feel like myself or the Fedora Project is rushing you. For now what I think we'll do in the Fedora package is skip that test just so that we can get a Python 3.13 build completed, even if it isn't perfect. If there are any commits you feel are really important for Python 3.13 compatibility, even if not final or included in a tagged release yet, I can include those as patches in the package. I can even include all of #2003 as a patch if you would like. We have several options here. We can always refresh the patch or even update to a new version after the Fedora 41 release (it isn't frozen like Debian).

@carlwgeorge
Copy link
Contributor Author

Closing the loop here, since #2003 was merged I went ahead and included it as a patch to the Fedora package. That let us complete a Python 3.13 build, which will unblock the installation of several other packages. Thanks!

@PeterJCLaw
Copy link
Collaborator

For info: you may also want to pull in #2008 too.

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.

2 participants