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

Type hinting in code #1749

Closed
penguinolog opened this issue Mar 24, 2017 · 30 comments
Closed

Type hinting in code #1749

penguinolog opened this issue Mar 24, 2017 · 30 comments

Comments

@penguinolog
Copy link
Contributor

Long story short

While working on big project, typing check helps to exclude shot in the leg scenario.

Expected behaviour

Type annotations on public methods.

Actual behaviour

No any type annotations, including the most docstrings.

Note

Need a decision: typing in global requirements or use version-specific requirements (PEP496/PEP508)

@penguinolog
Copy link
Contributor Author

@fafhrd91 Please comment decision, if possible.

@fafhrd91
Copy link
Member

Personally, I do not use type annotations. But I am fine with type annotations on public methods

@penguinolog
Copy link
Contributor Author

I'm about requirements on typing library. We can require it on every python version and can specify 3.4 only due to it presence in 3.5+ out of box.

@fafhrd91
Copy link
Member

That's fine

@iceboy233
Copy link
Member

I personally dislike type annotations in performance sensitive code in python. Any footprint bloat would slow things down, especially in interpreted languages.

@fafhrd91
Copy link
Member

fafhrd91 commented Apr 6, 2017

I thought there should be no runtime penalty?

@iceboy233
Copy link
Member

iceboy233 commented Apr 6, 2017

the annotations are accessible at runtime through __annotations__, so it will increase the memory footprint, allocator load (for the hash table for __annotations__), incref for the type objects, parsing time, etc.

@fafhrd91
Copy link
Member

fafhrd91 commented Apr 6, 2017

Ah, but compared to overall speed of python that doesn't really matter. I guess it is possible to come up with method how to strip annotations data at startup

@iceboy233
Copy link
Member

The annotations are not necessarily for type hints and are runtime visible. It's just the IDEs using them for type hints. I have used annotation for other things in my programs, such as param sanitizing.

@asvetlov
Copy link
Member

Annotations are perfect concept but before applying it to aiohttp we need reliable type checker tool.
The only existing tool for now is mypy and it's not production ready yet.

At least I had big problems when tried to check annotations for smaller aio-libs projects.
Let's postpone the issue (without closing).

@thehesiod
Copy link
Contributor

extra datapoint: all the code I write has type hints as it greatly helps finding issues in pycharm while developing

@kxepal
Copy link
Member

kxepal commented May 25, 2017

I also had negative experience with type hints as annotations. Tools are not ready indeed and you have to choose between two evils: do additional imports for getting types and accidentally end with circular ones or double number of files to maintain. So far good old sphinx docstrings are more handy and PyCharm can use it for type hinting. You still have to write docs anyway.

However, I think you can start make annotations today as typeshed part or as standalone project with stubs like pyspark and contribute back fixes of any problems you found.

@asvetlov
Copy link
Member

python/typing#84 is a blocker: without support for third-party libs in mypy type annotations are useless for aiohttp users.

@asvetlov
Copy link
Member

I don't want to support aiohttp stubs in typeshed but want to add annotations into aiohttp itself

@penguinolog
Copy link
Contributor Author

I have requirements concern:
typing is available starting from Python 3.5, backport can be required using PEP0508 (incorrectly parsed by some packagers), PEP0426 (deprecated way, totally ignored by some packagers) or patching requirements list in setup.py (bad looking way).

@asvetlov
Copy link
Member

I think we'll drop 3.4 before adding type hints

@asvetlov asvetlov changed the title Typing hints in code [on hold] Typing hints in code Sep 17, 2017
@chrisimcevoy
Copy link

Can the "on hold" status be revoked now that 3.0 is out the door?

@webknjaz
Copy link
Member

@chrisimcevoy is fairly hard to add typing everywhere instantly, but you are welcome to help with this by adding bit of type annotations in PRs ;)

@asvetlov
Copy link
Member

How aiohttp 3.0 release is related to PEP 561 implementation?
mypy tool doesn't support it yet.
I don't want adding types without checking them like we don't write a code without adding unittests.
Maybe it is an acceptable practice for other projects but aiohttp assumes high code quality.

@chrisimcevoy
Copy link

@webknjaz Agreed. Gradual typing would be best. I would like to find time for a small-ish PR if it would aid discussion and get the ball rolling.

@asvetlov Apologies, I should have been more clear. I was referring to your earlier comment, that dropping Python 3.4 support was a prerequisite for type hinting aiohttp. If mypy support for PEP 561 is also a blocker, then so be it.

@asvetlov
Copy link
Member

I hope mypy issue will be solved soon.

@asvetlov asvetlov changed the title [on hold] Typing hints in code [on hold] Type hinting in code Feb 27, 2018
@asvetlov asvetlov mentioned this issue Feb 27, 2018
5 tasks
@asvetlov
Copy link
Member

asvetlov commented Mar 5, 2018

Blockers: python/mypy#4255 and python/mypy#4403

@asvetlov
Copy link
Member

python/mypy#4693 is a new blocker

@asvetlov
Copy link
Member

@thomaszdxsn
Copy link
Contributor

awesome

@asvetlov
Copy link
Member

We need to wait for mypy 0.590 release, after that setup mypy tool on travis and start adding annotations.
Multidict and Yarl should be adopted first.

@samuelcolvin
Copy link
Member

v0.590 and v.600 are both released, what's the next step for this?

@asvetlov
Copy link
Member

asvetlov commented Jun 4, 2018

I have a #3049
Not done yet, make mypy generates about a dozen errors.
Next mypy release should fix two of them (I've added typeshed stubs for netrc and sys.implementation.

Others should be fixed in the branch before merging.

The branch has a limited typing support, but after the merge we can do the rest in separate iterations.

@asvetlov asvetlov changed the title [on hold] Type hinting in code Type hinting in code Jun 18, 2018
@asvetlov
Copy link
Member

The challenge is huge.
#3049 adds a part of annotations.
Others should be done in separate PRs, champions are welcome.

@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.
Projects
None yet
Development

No branches or pull requests

10 participants