python / typeshed

Collection of library stubs for Python, with static types
Other
4.39k stars 1.77k forks source link

Use of generics in requests typing #10899

Open multimeric opened 1 year ago

multimeric commented 1 year ago

Currently the requests typing makes heavy use of unions. For example the parameter type is: https://github.com/python/typeshed/blob/f99e8b0615182bb0f17d9d126019d8d72c207ab3/stubs/requests/requests/sessions.pyi#L97-L103

The issue with this approach is that the type system can't keep track of which type has been chosen for a given field. For example, running pyright on this code:

from requests import Session

session = Session()
session.params = {}
reveal_type(session.params)

Shows us that the type system immediately "forgets" that our parameters are definitely in a dictionary:

test.py:5:13 - information: Type of "session.params" is "dict[str | bytes | int | float, str | bytes | int | float | Iterable[str | bytes | int | float] | None]"
0 errors, 0 warnings, 1 information

If we could make Session (as well as other classes such as Request) generic over the parameter type (as well as other fields), then we could keep track of this kind of state. For example, it might look like:

session: Session[dict] = Session()
AlexWaygood commented 1 year ago

Feel free to file a PR trying this out! My experience with our requests stubs is that changing literally anything usually breaks the world -- but if you file a PR, mypy_primer should hopefully give us a sense about whether or not the change is likely to be disruptive :-)

srittau commented 1 year ago

I fear that this will have to wait until type var defaults PEP 696 become a reality, because it's likely that it's too disruptive. But as @AlexWaygood wrote, a PR to judge the effect can't hurt.

JelleZijlstra commented 1 year ago

I agree that this is impractical without TypeVar defaults, and possibly even with them. Just looking at the definition of requests.sessions.Session, we could easily make it generic over seven or so type variables:

    headers: MutableMapping[str, str | bytes]
    auth: _Auth | None
    proxies: _TextMapping
    hooks: dict[str, list[_Hook] | Any]
    params: _Params
    verify: _Verify | None
    cert: _Cert | None

And then you'd have to write Session[dict[str, str], None, dict[str, str], dict[str, list[...]], dict[str, str], None, None], which would be extremely user-unfriendly.

multimeric commented 1 year ago

I was under the impression that I could set type var defaults by overriding the __new__ return type, but this doesn't seem to actually work: https://github.com/multimeric/typeshed/blob/b82e2bf8dd00ad9a74265934cf01027bfa7dcd21/stubs/requests/requests/sessions.pyi#L138

from requests import Session

session = Session()
reveal_type(session)
reveal_type(session.params)
session.params = "q=foo&r=bar"
reveal_type(session.params)
reveal_type(session)

Gives me:

test.py:4: note: Revealed type is "requests.sessions.Session[Any, Any, Any]"
test.py:5: note: Revealed type is "Any"
test.py:7: note: Revealed type is "builtins.str"
test.py:8: note: Revealed type is "requests.sessions.Session[Any, Any, Any]"