From 4b8774041d8d509a70c5636f3c9835d333943234 Mon Sep 17 00:00:00 2001 From: Changsheng Date: Fri, 2 Oct 2020 11:34:57 -0400 Subject: [PATCH] Allow covariants of __enter__, __aenter__, and @classmethod (#1336) * Allow covariants of __enter__, __aenter__, and @classmethod The problem we currently have is the return type of classes such as Client does not allow covariants when subclassing or context manager. In other words: ```python class Base: def __enter__(self) -> Base: # XXX return self class Derived(Base): ... with Derived() as derived: # The type of derived is Base but not Derived. It is WRONG ... ``` There are three approaches to improve type annotations. 1. Just do not type-annotate and let the type checker infer `return self`. 2. Use a generic type with a covariant bound `_AsyncClient = TypeVar('_AsyncClient', bound=AsyncClient)` 3. Use a generic type `T = TypeVar('T')` or `Self = TypeVar('Self')` They have pros and cons. 1. It just works and is not friendly to developers as there is no type annotation at the first sight. A developer has to reveal its type via a type checker. Aslo, documentation tools that rely on type annotations lack the type. I haven't found any python docuementation tools that rely on type inference to infer `return self`. There are some tools simply check annotations. 2. This approach is correct and has a nice covariant bound that adds type safety. It is also nice to documentation tools and _somewhat_ friendly to developers. Type checkers, pyright that I use, always shows the the bounded type '_AsyncClient' rather than the subtype. Aslo, it requires more key strokes. Not good, not good. It is used by `BaseException.with_traceback` See https://github.com/python/typeshed/pull/4298/files 3. This approach always type checks, and I believe it _will_ be the official solution in the future. Fun fact, Rust has a Self type keyword. It is slightly unfriendly to documentation, but is simple to implement and easy to understand for developers. Most importantly, type checkers love it. See https://github.com/python/mypy/issues/1212 But, we can have 2 and 3 combined: ```python _Base = typing.TypeVar('_Base', bound=Base) class Base: def __enter__(self: _Base) -> _Base: return self class Derive(Base): ... with Derived() as derived: ... # type of derived is Derived and it's a subtype of Base ``` * revert back type of of SteamContextManager to Response * Remove unused type definitions * Add comment and link to PEP484 for clarification * Switch to `T = TypeVar("T", covariant=True)` * fixup! Switch to `T = TypeVar("T", covariant=True)` * Add back bound=xxx in TypeVar Co-authored-by: Florimond Manca --- httpx/_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index 5bbcc86237..74cee4f1dc 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -56,6 +56,12 @@ warn_deprecated, ) +# The type annotation for @classmethod and context managers here follows PEP 484 +# https://www.python.org/dev/peps/pep-0484/#annotating-instance-and-class-methods +T = typing.TypeVar("T", bound="Client") +U = typing.TypeVar("U", bound="AsyncClient") + + logger = get_logger(__name__) KEEPALIVE_EXPIRY = 5.0 @@ -1106,7 +1112,7 @@ def close(self) -> None: if proxy is not None: proxy.close() - def __enter__(self) -> "Client": + def __enter__(self: T) -> T: self._transport.__enter__() for proxy in self._proxies.values(): if proxy is not None: @@ -1752,7 +1758,7 @@ async def aclose(self) -> None: if proxy is not None: await proxy.aclose() - async def __aenter__(self) -> "AsyncClient": + async def __aenter__(self: U) -> U: await self._transport.__aenter__() for proxy in self._proxies.values(): if proxy is not None: