chore: add mypy

[miketheman]

Starts #10865

See each commit for reasoning and descriptions.

Check out this branch, run pip install -r requirements/lint.txt && bin/lint for a nice success.

Signed-off-by: Mike Fiedler [email protected]

[miketheman]

I’ll correct the failing lint tomorrow. Feels weird to have to move lines to allow for an ignore, so I may also ignore the flake8 E501 on these two lines as well.

[miketheman]

@di I think this is good to go - I didn't add mypy to bin/lint just yet, but could since it's passing right now.

[di]

This LGTM, I'll let @ewdurbin review as well. Nice to see this already catching bugs.

[ewdurbin]

I don't feel like I have enough understanding here to explicitly approve this 😅.

What configuration is allowing us to implement this gradually? Will mypy detect when files have changed, or does it just skip over unannotated code?

The only other concern I have here is the long list of explicitly untyped modules, is there some way for us to know when typing is added or stubs are available? Feels like it will fall on those motivated to keep track and send PRs which I suppose is fine.

I'm not at all against merging this and starting down the path, I'm interested to see what the impact on workflow will be for myself and other contributors!

[miketheman]

I'll try to clarify as best as I understand!

What configuration is allowing us to implement this gradually?

The config as I've started will warn if we added mypy configs that aren't used (useful to prevent config bloat), and examine every file in scope for potential inconsistencies, using type inference.

The fix made in 69a07b9 (#10874) is an example of this - the function signature declared that since: int and then redefined it inside the function scope to a datetime, so that triggered a mypy alert of:

warehouse/legacy/api/xmlrpc/views.py:550: error: Incompatible types in assignment (expression has type "datetime", variable has type "int")

If we wanted to allow this behavior, we would configure --allow-redefinition (ref) - but since there was a single instance in the entire codebase, it felt like a good opportunity to fix.

While this is not a bug per se, since the code works, and Python allows redeclaration of a variable, mypy points out that this is a confusing behavior to the reader of the code, so a rename to a more explicit variable helps comprehension and solves the error.

Will mypy detect when files have changed, or does it just skip over unannotated code?

Yes! By running each time with CI, we lint everything in the warehouse module. (We could also lint tests/ but that's likely overkill.)

We can opt in to stricter configurations one at a time, preventing a huge PR, and then finally flip the switch to be --strict and prevent things from entering the codebase at that time.

Here's a list of flags under --strict:

Strict mode; enables the following flags: --warn-unused-configs, --disallow-any-generics, --disallow-subclassing-any, --disallow-untyped-calls, --disallow-untyped-defs, --disallow-incomplete-defs, --check-untyped-defs, --disallow-untyped-decorators, --no-implicit-optional, --warn-redundant-casts, --warn-unused-ignores, --warn-return-any, --no-implicit-reexport, --strict-equality

So I'd advise taking on each one, adding to the config, running the lint, fixing/confirming, and shipping changes in iterative fashion.

The only other concern I have here is the long list of explicitly untyped modules, is there some way for us to know when typing is added or stubs are available? Feels like it will fall on those motivated to keep track and send PRs which I suppose is fine.

Not really, which is why I've added PR links that I could find for projects that may add typing in the future.

The typeshed project collects and maintains a lot of types-* packages, but it's really up to the package's to ship a py.typed file to signal they have type information, per PEP-561.

One excellent way to reduce the amount of untyped packages in scope is to... reduce the total number of packages in use. 😁 But barring that, it's reasonable to check back in on packages as part of their upgrade cycles.

Let me know if this makes sense!

[ewdurbin]

Yes. Thanks for the clarification @miketheman! @di I'll let you push the big green button.