Skip to content

Add mixins module to asyncio#6789

Merged
srittau merged 10 commits intopython:masterfrom
sobolevn:patch-59
Jan 25, 2022
Merged

Add mixins module to asyncio#6789
srittau merged 10 commits intopython:masterfrom
sobolevn:patch-59

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.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

This comment has been minimized.

Sign in to view
sobolevn
sobolevn commented Jan 2, 2022
View reviewed changes
stdlib/VERSIONS
ast: 2.7-
asynchat: 2.7-
asyncio: 3.4-
asyncio.mixins: 3.10-
Copy link
Member Author

@sobolevn sobolevn Jan 2, 2022

Choose a reason for hiding this comment

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

stubtest is still failing for <3.9 🤔
https://github.com/python/typeshed/runs/4686388253?check_suite_focus=true

Copy link
Collaborator

@srittau srittau Jan 25, 2022

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

@srittau srittau Jan 25, 2022

Choose a reason for hiding this comment

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

python/mypy#12062

@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.

Sign in to view
@sobolevn sobolevn closed this Jan 25, 2022
@sobolevn sobolevn reopened this Jan 25, 2022
@github-actions

This comment has been minimized.

Sign in to view
AlexWaygood
AlexWaygood reviewed Jan 25, 2022
View reviewed changes
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.

Sign in to view
2 similar comments
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@srittau srittau mentioned this pull request Jan 25, 2022
Closed
srittau
srittau reviewed Jan 25, 2022
View reviewed changes
stdlib/asyncio/mixins.pyi Outdated
_marker: object

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

@srittau srittau Jan 25, 2022

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.

srittau
srittau approved these changes Jan 25, 2022
View reviewed changes
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

sobolevn commented Jan 25, 2022

@AlexWaygood sure, in an hour 👍

@github-actions

This comment has been minimized.

Sign in to view
@sobolevn
Copy link
Member Author

sobolevn commented Jan 25, 2022 •
edited
Loading

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

github-actions bot commented Jan 25, 2022

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

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@srittau srittau srittau approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sobolevn @srittau @AlexWaygood