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

Add type annotations to urllib3 #867

Closed
tharvik opened this issue May 19, 2016 · 20 comments
Closed

Add type annotations to urllib3 #867

tharvik opened this issue May 19, 2016 · 20 comments

Comments

@tharvik
Copy link
Contributor

tharvik commented May 19, 2016

Would that be okay if type annotations of urllib3 were added to the typeshed repo?

It would be nice to have your explicit consent, as stated in PEP 484.

@haikuginger
Copy link
Contributor

I personally have no objection, although it'd probably be best for us to double-check the annotation when complete.

@tharvik
Copy link
Contributor Author

tharvik commented May 19, 2016

I'll write some and let you know when I open the PR, thanks!

@Lukasa
Copy link
Sponsor Contributor

Lukasa commented May 19, 2016

I also have no objection, though part of me wonders: if the type checker is now good enough, should we just have stub files here and make them part of our CI?

@tharvik
Copy link
Contributor Author

tharvik commented May 19, 2016

Function annotation are only available in python 3, you may want to keep compatibility with python 2 then.

@Lukasa
Copy link
Sponsor Contributor

Lukasa commented May 19, 2016

@tharvik That's why I'm suggesting stub files: stub files can be distributed with the library without breaking Python 2.

@tharvik
Copy link
Contributor Author

tharvik commented May 19, 2016

Ho, misread, yup, that would be better this way, I'll do you a pull request soon then (and by design of typeshed, also copy it there).

@Lukasa
Copy link
Sponsor Contributor

Lukasa commented May 19, 2016

My understanding of typeshed is that actually they'd rather we distributed it than they did. They shouldn't really need a copy if we ship one.

@tharvik
Copy link
Contributor Author

tharvik commented May 19, 2016

Correct, I'm reading PEP 484 again.

From it, we can either have stubs in shared/typehints/python* or alongside the existing python files. What do you prefer?

@tharvik
Copy link
Contributor Author

tharvik commented May 19, 2016

From what I just saw, it would be easier to split it by python version, because of much conditional imports and behaviour.

@Lukasa
Copy link
Sponsor Contributor

Lukasa commented May 19, 2016

I'm happy to split by Python version, at least in the first instance.

@sigmavirus24
Copy link
Contributor

mypy-lang supports using the magical comment annotations, why not use that instead?

@sigmavirus24
Copy link
Contributor

if the type checker is now good enough

@Lukasa mypy-lang is good, yes. But the typing module is still as it was when it was introduced in PEP 484 (primitive).

@shazow
Copy link
Member

shazow commented May 19, 2016

(No objections here)

@haikuginger haikuginger changed the title add urllib3 to typeshed Add type annotations to urllib3 May 19, 2016
@tharvik
Copy link
Contributor Author

tharvik commented May 20, 2016

mypy-lang supports using the magical comment annotations, why not use that instead?

@sigmavirus24 we can also do that, but for now, mypy is still quite bad at handling conditional imports, so if we want it to be useful now, it would be better to have splitted stubs.

@and-semakin
Copy link

At the current moment mypy is mature and should be considered stable. There are alternative type checkers available such as pyright and pyre. I believe overall Python typing ecosystem is developed enough. Of course there are still some questions to be answered.

urllib3 is the most popular package according to PyPI stats and it is lacking of type hints. I'm pretty sure that adding typing here will evolve all the Python ecosystem. It is definitely worth the effort.

As far as I can see there was an attempt (#880) to add type hints. I think that we should take one more attempt to include it here. I can try to do it for my own or help someone who wants to coordinate the process. What do you think? @tharvik?

@sethmlarson
Copy link
Member

I'm 100% on adding type-hints, just needs someone to execute the work. :)

@haikuginger
Copy link
Contributor

I'm also 100% in favor of adding type annotations now. I'd suggest that we do so progressively on our external interfaces (PoolManager methods?) first and then move inwards for additional validation of our more private surfaces.

@shazow
Copy link
Member

shazow commented Nov 4, 2019

(Still no objections here)

@sethmlarson sethmlarson added the v2 label Jun 15, 2020
gladhorn added a commit to ollo69/wideq that referenced this issue Jun 28, 2020
There are no annotations for it, see
urllib3/urllib3#867

This should make the mypy check happy.
@pquentin
Copy link
Member

Should this be closed in favor of #1897?

@sethmlarson
Copy link
Member

Yeah, closing this one :)

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

No branches or pull requests

8 participants