Start shipping our own type stubs

[sethmlarson]

Typeshed houses very rudimentary type stubs for urllib3 but they're hidden within requests.packages.urllib3 so Mypy can't take advantage of them when using urllib3 on its own. We could also probably improve on what is already in typeshed by adding better types than Any everywhere.

[savarin]

Hi @sethmlarson! I'm at Recurse Center and got introduced to his by @shazow. Could I clarify, as per the title, what you have in mind is to have the .pyi files alongside .py files? Would the first step simply involve porting the one in requests, and then expanding on the Any?

I've actually been a little stuck getting the stubs hidden in requests to work with mypy, even after importing requests.

For additional context, I spent some time in my previous making py2 code compatible with py2/py3 and adding type comments. The thought was there's less friction not having to review separate .pyi files when refactoring.

[sethmlarson]

@savarin Glad you're interested in helping us here. I was thinking starting with the type stubs that typeshed has (pyi) and making the Any more specific. We can do this work incrementally, to be easily reviewable.

Let me know if you have additional questions!

[judej]

@sethmlarson, would you prefer that we modify the stubs in typeshed or would you rather have the .pyi files in this repo checked in alongside the .py files? OR, would you rather add the type info into the .py files?

[sethmlarson]

I'd like the pyi files shipped with urllib3.

[savarin]

Cool, sounds like a good starting point. Two questions, stemming from my lack of familiarity with stubs and/or typeshed.

1. How do you run a type check with the existing typeshed stubs of urllib3 since it's in requests? Detailed run below, importing requests doesn't seem to help.

$ cat run.py

import requests
import urllib3

def pool():
    return urllib3.PoolManager()

$ mypy run.py
run.py:3: error: Skipping analyzing 'urllib3': found module but no type hints or library stubs
run.py:3: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

2. I presume all the changes would be in the typeshed repo alone. What's the anticipated flow to check these changes are correct (since typeshed normally comes with mypy), i.e. clone typeshed, move to MYPYPATH, run mypy on urrlib3 tests (since stubs won't check the code itself)?

[pquentin]

+1 for shipping our own type stubs. .pyi files aren't ideal, but types as comments aren't either. Hopefully in 2021 we'll be able to drop Python 2 support in urllib3 2.0.

I have one question though: does anyone understand how providing types for requests.packages.urllib3 in typeshed achieves anything?

urllib3 used to be vendored in requests, but it's no longer the case: requests now directly imports urllib3, without going through requests.package.urllib3 which only exists for backwards-compatibility: it allows others to continue importing urllib3 through requests.package.

[sethmlarson]

@pquentin I'm guessing it's for backwards compatibility :)

@savarin All changes would be made in the urllib3 repo, not typeshed. I'd recommend the following approach:

• Add the type stubs from typeshed to urllib3 and add necessary changes to ensure pyi files are added to dists

• File by file update all Any usages with specific types

• 🎉 Profit!

[savarin]

urllib3 used to be vendored in requests, but it's no longer the case

@pquentin Ah this makes more sense now, nullifies questions I had above.

@sethmlarson This is helpful, thank you! After the port, what would the check for correctness be? As above, my understanding is stubs can't run checks on the code so I presume it'll just be on urllib3/test.

[sethmlarson]

Mypy can check that types are consistent within themselves.

Maybe the approach I took with Eland could be useful for us? https://github.com/elastic/eland/blob/master/noxfile.py#L75

[savarin]

Including the dependency tree from #1924 to this issue.

[savarin]

@pquentin Didn't mean to cause confusion deleting a prior comment. I'm getting some strange behavior (e.g. not getting a mypy error when I comment out 33c4c13, even after introducing a breakpoint and looking through all the errors) which I can't quite explain. I suspect it's something to do with the mypy cache, will fiddle around a bit more.

[pquentin]

Agreed that something weird is going on. I'm not familiar enough with mypy to help, sorry!

[savarin]

@pquentin No worries, just wanted to check it's something you're seeing as well.

I've tested this out a bit more, as discussed commenting out 33c4c13 doesn't trigger any mypy errors with nox -s lint. I got it to trigger by importing creating a file that imports from _implementation.py (compare test passing in 41f3957 vs failing in 207a569). There's a discussion on a similar issue here.

@sethmlarson Let me know if you have any thoughts. I'll continue adding stubs and testing it on toy imports where possible. It's a little unsettling that the existing mypy tests aren't raising errors (without the toy import).

[djbrown]

@hramezani how does #2315 fix this issue?
Where do I find and install urllib3 type stubs? 🤔

[KapJI]

They will be released with v2.

[pquentin]

Having types ready is great news! However v2.0 is still months away. If anyone wants to help, we have bounties on various issues blocking the releases: https://github.com/urllib3/urllib3/milestone/6. They're not good first issues but we're here to help.

If you're interested, consider talking to us on Discord (https://discord.gg/urllib3) or in the issues themselves!