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

WIP: Add type hints everywhere #3280

Merged
merged 26 commits into from
Sep 26, 2018
Merged

WIP: Add type hints everywhere #3280

merged 26 commits into from
Sep 26, 2018

Conversation

asvetlov
Copy link
Member

Enable strong mypy checks to make sure that types are correct.

@kornicameister
Copy link
Contributor

@asvetlov I've added #3284 (not working yet, but wanted to fix up later) , but given this PR I assume mine overlaps yours. I think I will close my down unless you have other idea?

@asvetlov
Copy link
Member Author

I've merged your PR into master and this branch.
Thank for your work.

This PR enables mypy checks in strongest possible mode.
I hope this will allow getting rid of typing errors, especially improper annotations.

@kornicameister
Copy link
Contributor

@asvetlov happy to help. I am quite interested in getting this in since I am using both mypy and aiohttp in my project ;-). Those little errors in Neovim aren't necessary helping :D.

I will check out your PR and see if there's anything more to work on.
Or maybe I will find sth else ;-)

@kornicameister kornicameister mentioned this pull request Sep 24, 2018
5 tasks
@kornicameister
Copy link
Contributor

@asvetlov If you want, might be that #3289 is something you might want to pull in as well. I am not sure how the customs are here, but I usually prefer latest available libraries ;-).

@asvetlov
Copy link
Member Author

@kornicameister if you want to help we can do the following:

  1. I'll disable strict mypy mode (comment strict options in config) and merge the PR
  2. You can pick up the master branch, uncomment these settings and work on some file
  3. Your PR(s) will fail on CI, I'll check it manually and merge.

Adding types to all files is a big work, I appreciate any help

@kornicameister
Copy link
Contributor

kornicameister commented Sep 25, 2018

Yeah...why not 🤔. I will try to get sth as soon as I have some time off being dad 😃.

That said I will wait for you to merge this one. I suppose I can actually pick up this branch as a starting point for the being.

@kornicameister
Copy link
Contributor

@asvetlov FYI: starting with aiohttp/payload.py

@asvetlov
Copy link
Member Author

@kornicameister cool!
I'm about to make the PR green

@codecov-io
Copy link

codecov-io commented Sep 26, 2018

Codecov Report

Merging #3280 into master will increase coverage by 0.03%.
The diff coverage is 99.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3280      +/-   ##
==========================================
+ Coverage   97.99%   98.03%   +0.03%     
==========================================
  Files          43       43              
  Lines        7896     8001     +105     
  Branches     1354     1354              
==========================================
+ Hits         7738     7844     +106     
  Misses         65       65              
+ Partials       93       92       -1
Impacted Files Coverage Δ
aiohttp/web_protocol.py 92.02% <ø> (+0.27%) ⬆️
aiohttp/locks.py 100% <100%> (ø) ⬆️
aiohttp/resolver.py 100% <100%> (ø) ⬆️
aiohttp/http_writer.py 98.09% <100%> (+0.01%) ⬆️
aiohttp/pytest_plugin.py 98.63% <100%> (ø) ⬆️
aiohttp/cookiejar.py 100% <100%> (ø) ⬆️
aiohttp/abc.py 100% <100%> (ø) ⬆️
aiohttp/test_utils.py 99.27% <100%> (ø) ⬆️
aiohttp/streams.py 99.2% <100%> (ø) ⬆️
aiohttp/http_exceptions.py 100% <100%> (ø) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 044996a...6c3873d. Read the comment docs.

@asvetlov asvetlov merged commit da75122 into master Sep 26, 2018
@asvetlov asvetlov deleted the mypy branch September 26, 2018 21:15
@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
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.

4 participants