]> git.ipfire.org Git - thirdparty/httpx.git/commitdiff
Allow covariants of __enter__, __aenter__, and @classmethod (#1336)
authorChangsheng <Congee@users.noreply.github.com>
Fri, 2 Oct 2020 15:34:57 +0000 (11:34 -0400)
committerGitHub <noreply@github.com>
Fri, 2 Oct 2020 15:34:57 +0000 (16:34 +0100)
* 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 <florimond.manca@gmail.com>
httpx/_client.py

index 5bbcc8623797222494bc4c83fa8a74e7d35e8e86..74cee4f1dcd606d173a55f912431d2bcf36da781 100644 (file)
@@ -56,6 +56,12 @@ from ._utils import (
     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 @@ class Client(BaseClient):
                 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 @@ class AsyncClient(BaseClient):
                 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: