Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type hints: Add static type checker mypy #2950

Merged
merged 10 commits into from
Mar 17, 2023
Merged

Type hints: Add static type checker mypy #2950

merged 10 commits into from
Mar 17, 2023

Conversation

binste
Copy link
Contributor

@binste binste commented Mar 7, 2023

Adds mypy as a linter and fixes all current errors. This is a prerequisite for the subsequent tasks outlined in #2951.

@binste binste changed the title Add static type checker mypy Type hints: Add static type checker mypy Mar 7, 2023
@binste binste mentioned this pull request Mar 7, 2023
14 tasks
@binste binste marked this pull request as ready for review March 16, 2023 20:02
@mattijn
Copy link
Contributor

mattijn commented Mar 16, 2023

Thanks for this PR @binste!
Can we make use of the pipe operator introduced in 3.10 through typing_extensions?

Eg

Dict[str, Any] | None

Instead of

Optional[Dict[str, Any]]

@binste
Copy link
Contributor Author

binste commented Mar 17, 2023

Unfortunately no. The pipe operator throws a SyntaxError prior to 3.10, see https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-x-y-syntax-for-unions. This can be circumvented by doing the "Future Annotations Import" from __future__ import annotations which seems to be a highly debated topic in the Python world and support by default for this behaviour has been postponed a few times, see python/cpython#82786. I'd therefore suggest that we stick to Union and Optional to have the broadest possible compatibility.

@mattijn
Copy link
Contributor

mattijn commented Mar 17, 2023

That's unfortunate. It would improve readability of the type definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants