-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Typing updates #3928
Typing updates #3928
Conversation
aiohttp/client.py
Outdated
@@ -178,6 +178,8 @@ class ClientSession: | |||
'_request_class', '_response_class', | |||
'_ws_response_class', '_trace_configs') | |||
|
|||
_default_headers: CIMultiDict[str] |
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.
aiohttp still supports Python 3.5, this syntax doesn't work.
I'm open for discussing the drop of Python 3.5 in aiohttp 4.0 though.
If you want to proceed in this direction please create a new issue for discussion.
Otherwise please use Python 3.5 compatible syntax in the PR
Sorry, I'm not really interested in legacy support or participating in related discussions. If this cannot be merged right away, it should either hang until it can be merged, or have only compatible bits cherry picked, or just be discarded. |
@AMDmi3 it fails even under Python 3.7: https://ci.appveyor.com/project/aio-libs/aiohttp/builds/26164328/job/0nn5x41jb9ow927u#L1023 |
Curious what Towncrier has to do with this... https://travis-ci.com/aio-libs/aiohttp/jobs/218370235#L527 |
@webknjaz I don't know but towncrier wants to import the served package for some reason. |
Ah right.. Package version. Makes sense. AFAIR you can use |
Actually I think there's problem with typing support in
|
Use forwarded type declaration (string) :
It is not multidict error, |
@asvetlov Isn't the issue that the C implementation of See MagicStack/immutables#13 and https://gitter.im/python/typing?at=5d25bd59cdb70805c96e239f for a similar issue. |
|
@asvetlov Yes, but currently using |
The way is using forward declarations: |
What do you mean with "the way"? In my opinion, a better way for the multidict project would be to implement |
Exactly. aiohttp should use strings here. |
- Add type annotations to global variables - Fix handling of unused return values from time.gmtime - Silence mypy error on return line (it's not yet capable of deducing that _cached_formatted_datetime is always defined at the end of the function)
(MatchObject.lastindex may be None)
...by moving field type annotation to class definition
...by moving field type annotation to class definition
...by moving field type annotation to class definition
error: Incompatible types in "await" (actual type "Generator[Any, None, AbstractServer]", expected type "Awaitable[Any]") Not sure how to fix, may be a typeshed problem
Not sure how to fix this properly, the logic is hard to follow
Switched to string annotations, python 3.6+ build seem to be fixed |
Outdated, the master supports newer mypy without errors |
What do these changes do?
Bump mypy version to 0.720 and fix/silence new mypy issues.
Are there changes in behavior for the user?
No
Related issue number
#3927
Checklist
CONTRIBUTORS.txt
CHANGES
folder (may do after more changes are merged)<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.