FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix loop.remove_reader() and loop.remove_writer() signatures by asvetlov · Pull Request #7042 · python/typeshed · GitHub

Fix loop.remove_reader() and loop.remove_writer() signatures - #7042

Merged
srittau merged 2 commits into
python:masterfrom
asvetlov:fix-remove-reader-writer
Jan 26, 2022
Merged

Fix loop.remove_reader() and loop.remove_writer() signatures#7042
srittau merged 2 commits into
python:masterfrom
asvetlov:fix-remove-reader-writer

Conversation

Copy link
Copy Markdown
Contributor

These methods return True if reader/writer was previously set, False otherwise.

Copy link
Copy Markdown
Contributor

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

srittau commented Jan 26, 2022

Copy link
Copy Markdown
Collaborator

We should also change the return type in AbstractEventLoop.

Copy link
Copy Markdown
Contributor Author

Correct, thanks. Fixed.

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

tornado (https://github.com/tornadoweb/tornado)
+ tornado/platform/asyncio.py:641: error: Return type "None" of "remove_reader" incompatible with return type "bool" in supertype "AbstractEventLoop"
+ tornado/platform/asyncio.py:645: error: Return type "None" of "remove_writer" incompatible with return type "bool" in supertype "AbstractEventLoop"

asvetlov commented Jan 26, 2022
edited
Loading

Copy link
Copy Markdown
Contributor Author

Tornado is slightly incompatible with the reference asyncio implementation, sorry.
The boolean was returned from these methods starting from the very first asyncio release, please trust me :)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

2 participants


Back | FazBrowse Home | New Git URL