Skip to content

Add mixins module to asyncio #6789

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

Merged
merged 10 commits into from
Jan 25, 2022
Merged

Add mixins module to asyncio #6789

merged 10 commits into from
Jan 25, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 2, 2022

No description provided.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 2, 2022

I am checking what CI will say about adding module to VERSIONS, but not to the module. Source: https://github.com/python/cpython/commits/main/Lib/asyncio/mixins.py

@sobolevn
Copy link
Member Author

sobolevn commented Jan 2, 2022

This module does not have any public objects, but we can still add several protected ones. I don't know how they are supposed to be used though.

@sobolevn sobolevn marked this pull request as ready for review January 2, 2022 22:41
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@@ -62,6 +62,7 @@ array: 2.7-
ast: 2.7-
asynchat: 2.7-
asyncio: 3.4-
asyncio.mixins: 3.10-
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sobolevn In theory, mypy should support this: python/mypy#11069, but in practice, asyncio.exceptions is also ignored in the stubtest allowlists. You need to do the same with asyncio.mixins for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sobolevn
Copy link
Member Author

sobolevn commented Jan 4, 2022

@srittau @Akuli can you please help me? I don't understand why tests fail:


Run python tests/stubtest_stdlib.py
error: asyncio.mixins failed to find stubs
Stub:
MISSING
Runtime:
None

It happens on all python versions <3.10, when this module was introduced. But, I've marked it appropriately in VERSION file 🤔

@srittau srittau closed this Jan 7, 2022
@srittau srittau reopened this Jan 7, 2022
@github-actions

This comment has been minimized.

@sobolevn sobolevn closed this Jan 25, 2022
@sobolevn sobolevn reopened this Jan 25, 2022
@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Could you update the classes in asyncio.locks and asyncio.queues so that they inherit from _LoopBoundMixin, as they do at runtime?

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

_marker: object

class _LoopBoundMixin:
def __init__(self, *, loop: Any = ...) -> None: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should just skip the loop argument, It can't really be used. Then we can also remove _marker above.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

See my comment above, but I'm fine either way.

@sobolevn
Copy link
Member Author

@AlexWaygood sure, in an hour 👍

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 25, 2022

I went for loop: NoReturn. I think that it can be the best solution:

  1. It should not require any stubtest hacks
  2. It indicates that loop is not allowed

And also, looks like the change proposed by @AlexWaygood will require some extra tweaks, so I am going to leave this for some next PRs (while thinking about the best way to implement it).

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit bfda5c6 into python:master Jan 25, 2022
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 this pull request may close these issues.

3 participants