Stricter typing. #2436

Activity

1. lovelydinosaur

   added

   user-experienceEnsuring that users have a good experience using the libraryEnsuring that users have a good experience using the library

   toolingChanges to our CI/CD, tests setup, etc.Changes to our CI/CD, tests setup, etc.

   on Nov 11, 2022
2. mjpieters commented on Nov 11, 2022

   

   mjpieters

   on Nov 11, 2022

   Last edited by mjpieters

   Contributor

   More actions

   The project is in a pretty good shape, I must say. Out of the box, you can enable these settings already:

   • warn_unused_configs = True
   • disallow_subclassing_any = True
   • check_untyped_defs = True
   • disallow_untyped_decorators = True
   • warn_redundant_casts = True
   • strict_concatenate = True

   and scripts/check passes cleanly with no other changes required.

   I'd enable warn_unused_ignores = True too, and remove the 10 type: ignore comments that that then highlights. Those comments do nothing, according to mypy. It's a basic code hygiene check, like Flake8 telling you about unused imports.

   That leaves you with, in order of error count, with:

   • strict_equality = True - 2 errors
   • disallow_incomplete_defs = True - 7 errors
   • warn_return_any = True - 12 errors
   • no_implicit_reexport = True - 36 errors
   • disallow_untyped_calls = True - 92 errors

   and, of course

   • disallow_any_generics = True - 17 errors

   for which my PR has the required changes already. Going by those counts should make it easier to prioritise where to focus your efforts.

   Note that some of the reported errors are closely related. The changes in my PR also take care of 5 of the 12 errors for warn_return_any, because the type checker can then follow types flow through the code better and so can see that a specific piece of code doesn't return Any but a concrete type, so take the error numbers as a maximum.

   Finally, you could split enabling flags between the library itself, and the test suite. Those 92 errors for disallow_untyped_calls may look daunting, but they are all, without exception, in the test suite, so you can enable this flag in the main mypy configuration, and add disallow_untyped_calls = False under [mypy-tests.*] and get a clean bill of health immediately.
3. mjpieters commented on Nov 11, 2022

   

   mjpieters

   on Nov 11, 2022

   Last edited by mjpieters

   Contributor

   More actions

   [ ] no_implicit_reexport = True - 36 errors

   The error count here is misleading, actually. It requires just 4 changes to fix these:

   --- a/httpx/_compat.py
   +++ b/httpx/_compat.py
   @@ -14,10 +14,12 @@ if typing.TYPE_CHECKING:  # pragma: no cover
    # The C bindings in `brotli` are recommended for CPython.
    # The CFFI bindings in `brotlicffi` are recommended for PyPy and everything else.
    try:
   -    import brotlicffi as brotli
   +    import brotlicffi
   +
   +    brotli = brotlicffi
    except ImportError:  # pragma: nocover
        try:
   -        import brotli
   +        import brotli as brotli
        except ImportError:
            brotli = None
   --- a/tests/conftest.py
   +++ b/tests/conftest.py
   @@ -15,7 +15,7 @@ from cryptography.hazmat.primitives.serialization import (
        load_pem_private_key,
    )
    from uvicorn.config import Config
   -from uvicorn.main import Server
   +from uvicorn.server import Server
    
    from httpx import URL
    from tests.concurrency import sleep
   --- a/httpx/_models.py
   +++ b/httpx/_models.py
   @@ -41,7 +41,7 @@ from ._types import (
        ResponseExtensions,
        SyncByteStream,
    )
   -from ._urls import URL
   +from ._urls import URL as URL
    from ._utils import (
        guess_json_utf,
        is_known_encoding,

   If you applied my PR and the 4 changes above, and set disallow_untyped_calls = False for the test suite, you only have ~16 errors across 3 mypy flags left to tackle. The project is in a really good shape!
4. pmav99 commented on Nov 12, 2022

   

   pmav99

   on Nov 12, 2022

   More actions

   There is a minor typo in the diff: s/brotly/brotli/
5. mjpieters commented on Nov 12, 2022

   

   mjpieters

   on Nov 12, 2022

   Contributor

   More actions

   There is a minor typo in the diff: s/brotly/brotli/

   Well spotted! On my mac, the import brotli as brotli import was being used so I didn't notice. Original diff updated.
6. lovelydinosaur commented on Dec 2, 2022

   

   lovelydinosaur

   on Dec 2, 2022

   MemberAuthor

   More actions

   Okay, since #2479 we've now got mypy --strict passing.

   On that basis I'm going to close this ticket off.

   Here's a couple of optional follow-ups...

   • We could also tackle getting to --strict on the test cases too. If anyone's keen on tackling that, then I'd suggest opening a ticket similar to this one, enumerating which cases need to pass, and opening PRs on a case-by-case basis in the same way we've done here.
   • We could take a look at what errors pyright --strict httpx currently raises. Again, if anyone's keen on tackling that, then I'd suggest opening an issue describing the current pyright output.
7. lovelydinosaur

   closed this as completedon Dec 2, 2022