-
Notifications
You must be signed in to change notification settings - Fork 140
feat: support python 3.14 #1337
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
base: master
Are you sure you want to change the base?
feat: support python 3.14 #1337
Conversation
1a0d1f6
to
c538624
Compare
c538624
to
9bcfa83
Compare
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] | ||
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] | ||
os: ["windows-latest", "ubuntu-latest", "macos-latest"] | ||
experimental: [false] | ||
include: | ||
- python-version: '3.14.0-alpha - 3.14' | ||
experimental: true | ||
os: "ubuntu-latest" | ||
- python-version: '3.14.0-alpha - 3.14' | ||
experimental: true | ||
os: "windows-latest" | ||
- python-version: '3.14.0-alpha - 3.14' | ||
experimental: true | ||
os: "macos-latest" | ||
fail-fast: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would also work:
strategy:
matrix:
python:
- version: "3.8"
- version: "3.9"
- version: "3.10"
- version: "3.11"
- version: "3.12"
- version: "3.13"
- version: "3.14"
experimental: true
os: ["windows-latest", "ubuntu-latest", "macos-latest"]
fail-fast: true
(replacing .python-version
with .python.version
and .experimental
with .python.experimental
below)
try: | ||
return asyncio.get_running_loop() | ||
except RuntimeError: | ||
return asyncio.new_event_loop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return asyncio.new_event_loop() | |
asyncio.set_event_loop(loop := asyncio.new_event_loop()) | |
return loop |
(see https://github.com/python/cpython/blob/v3.12.11/Lib/asyncio/events.py#L699)
@@ -355,7 +355,7 @@ def before_invoke(self, coro: CFT) -> CFT: | |||
TypeError | |||
The coroutine passed is not actually a coroutine. | |||
""" | |||
if not asyncio.iscoroutinefunction(coro): | |||
if not disnake.utils.iscoroutinefunction(coro): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not disnake.utils.iscoroutinefunction(coro): | |
if not iscoroutinefunction(coro): |
finally: | ||
except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changes this method's behavior
@@ -123,9 +123,9 @@ async def _call_external_error_handlers( | |||
if local is not None: | |||
stop_propagation = await local(inter, error) | |||
# User has an option to cancel the global error handler by returning True | |||
finally: | |||
except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
with warnings.catch_warnings(): | ||
warnings.simplefilter("ignore", DeprecationWarning) | ||
self.loop: asyncio.AbstractEventLoop = asyncio.get_event_loop() | ||
self.loop: asyncio.AbstractEventLoop = utils.get_event_loop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar issue here, docstring still mentions asyncio.get_event_loop
.
except Exception: | ||
_log.exception("An error occurred while stopping the gateway. Ignoring.") | ||
return | ||
finally: | ||
self.stop() | ||
return # noqa: B012 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also changes behavior, continuing to heartbeat after closing the ws.
As noted in the PEP and B012, return
in finally
can swallow exceptions (in this case only BaseException
s), which I would argue is fine here; the thread is at the end of its lifecycle anyway.
@@ -1,5 +1,6 @@ | |||
# SPDX-License-Identifier: MIT | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | ||
_log.info("Probe found codec=%s, bitrate=%s", codec, bitrate) | ||
finally: | ||
return codec, bitrate # noqa: B012 | ||
_log.info("Probe found codec=%s, bitrate=%s", codec, bitrate) | ||
return codec, bitrate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else
part should've been fine here.
@@ -50,6 +50,22 @@ | |||
|
|||
from .enums import Locale | |||
|
|||
if sys.version_info >= (3, 11): | |||
from inspect import iscoroutine as iscoroutine, iscoroutinefunction as iscoroutinefunction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iscoroutine
isn't being used anywhere, as far as I can tell, and asyncio.iscoroutine
isn't deprecated.
Summary
Support python 3.14. All of our dependencies allow it at this point in time.
depends on #1328
Reviews/contributions are useful at any point in time.
Checklist
pdm lint
pdm pyright