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

Add Python 3.12 beta to test matrix #3751

Merged
merged 26 commits into from Jul 5, 2023
Merged

Add Python 3.12 beta to test matrix #3751

merged 26 commits into from Jul 5, 2023

Conversation

harshil21
Copy link
Member

@harshil21 harshil21 commented Jun 9, 2023

To be merged after #3728

Things changed for us:

@harshil21
Copy link
Member Author

harshil21 commented Jun 9, 2023

def gen():
    yield

await asyncio.create_task(gen())

does not work in python 3.12. Changing into an async def does not work either.

The 3.12 changelog notes:

"asyncio.iscoroutine() now returns False for generators as asyncio does not support legacy generator-based coroutines."

Does this mean we should drop support for generators in app.create_task too? Support for that was added in #3543 (comment).

Related code and PRs:

https://github.com/python/cpython/blob/main/Lib/asyncio/tasks.py#L106-L110
python/cpython#102749

harshil21 and others added 2 commits June 10, 2023 01:24
Also remove support for generators in app.create_task
@Bibo-Joshi
Copy link
Member

Does this mean we should drop support for generators in app.create_task too?

Not for py 3.11-, as that would be a breach of our stability policy. We can think about deprecating it, but dropping support unannounced is a no-go.
For 3.12+, to stay in line with our stability poclicy, we should try to still support it + add deprecation notices. If there is no feasible workaround, we ofc don't have a chance.

clot27 and others added 2 commits June 17, 2023 09:10
Co-authored-by: Aditya <clot27@apx_managed.vanilla>
>
@harshil21
Copy link
Member Author

@Bibo-Joshi okay, now how do I specify different type hints for the coroutine parameter for different python versions? Should I just use a if sys.version_info.. : @overload def create_task(...) thingy?

and what kind of workaround do you suggest for py3.12+? I don't see any

@Bibo-Joshi
Copy link
Member

@Bibo-Joshi okay, now how do I specify different type hints for the coroutine parameter for different python versions? Should I just use a if sys.version_info.. : @overload def create_task(...) thingy?

Sounds good if that works reasonably well. Don't know if it's possible to make only the overload part the content of the if clause or if you have to define the method twice

and what kind of workaround do you suggest for py3.12+? I don't see any

I don't have anything in mind TBH, haven't looked into it yet

Copy link
Member Author

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked by running mypy in different python versions, and also checked if VSCode shows a type error when passing an (async) generator to app.create_task for py > 3.12.

telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Looks like you edited the (optional) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the additional dependencies for the hooks in sync with the requirements :)

@harshil21
Copy link
Member Author

pre-commit.ci run

telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Show resolved Hide resolved
Base automatically changed from drop-py-37 to master June 29, 2023 16:17
@harshil21 harshil21 changed the title Add Python 3.12 beta 2 to test matrix Add Python 3.12 beta to test matrix Jul 2, 2023
@Bibo-Joshi Bibo-Joshi merged commit 589047d into master Jul 5, 2023
25 checks passed
@Bibo-Joshi Bibo-Joshi deleted the add-py-312 branch July 5, 2023 19:58
@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants