-
Notifications
You must be signed in to change notification settings - Fork 722
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
MNT: Avoid warning on 3.10.9+, 3.11.1+, and 3.12.0+ #1714
MNT: Avoid warning on 3.10.9+, 3.11.1+, and 3.12.0+ #1714
Conversation
I'm not a maintainer here but I do maintain the Fedora package of this library. I ran the tests with this PR on Python 3.12.0a4, and there is another exception.
To be clear the tests already don't pass on Python 3.12 on the master branch. |
That looks like the tests are leaking event loops.... |
The idea is to transition soon to using In the meantime, I can merge this and we can look at the leaking event loops in the tests. |
What do you think about dropping Python 3.6 support? It would simplify dealing with these issues. 3.6 is end of life for over a year now. That way, we can have |
As one of the authors of https://numpy.org/neps/nep-0029-deprecation_policy.html I am very 👍🏻 on dropping older Pythons (which as of last month says the support window is 3.8+)! I can see that maybe being a bit aggressive for prompttoolkit, but I think dropping support for versions of Python that are no longer supported by upstream should be completely non-controversial. |
I can not reproduce this. Bisecting the failure with py3.10.9 points to this commit as being the problem. Trying with py3.9.16 after this is merged I am also seeing things like
from pytest clean up which makes me think there is actually something quite wrong with this patch..... |
On a bit more consideration, I suspect this should be reverted in favor of eating the exception or a more complex fix. The problem is that Adding an The test that does the test suite in is I "tested" this by running the application where I was seeing the issue not by running the test suite 😞 . I suspect it passes on CI because the machines are slow enough that GC saves us or there is a high enough file limit set someplace. https://bugs.python.org/issue39529 / python/cpython#83710 are the upstream discussions of this. It seems the goal is to deprecate exactly this usage (to get a handle on the event loop before it is running, schedule some stuff, and then start it) so I am not sure there actually is a "good" path to this short of switching to |
Note that |
closes #1696