Enable mypy on Python #1041
Enable mypy on Python #1041
Conversation
| import logging | ||
|
|
||
| import fastavro | ||
| import fastavro # type: ignore |
There was a problem hiding this comment.
Is this going to be needed for every import that doesn't use type annotations?
There was a problem hiding this comment.
Yes for now, or we can disable it. More and more packages will have types in the future :)
| visit(write_schema, CheckCompatibility(read_schema, False)) | ||
|
|
||
| NO_ERRORS = [] | ||
| NO_ERRORS: List[str] = [] |
There was a problem hiding this comment.
Should this also be a tuple?
There was a problem hiding this comment.
There was a problem hiding this comment.
What I meant was: should this be tuple() because it is a constant and should not be mutable?
I am 👍 as well. I am not a huge fan of the visual distractions and verbosity of mypy annotations however once integrated into CI it should pull its weight. |
xhochy
left a comment
xhochy
left a comment
There was a problem hiding this comment.
+1, this definitely helps to detect some subtle errors. I would though set --ignore-missing-imports instead of the numerous # type: ignore on third-party imports.
|
@Fokko, could you use |
|
I've updated the tox command. I've also pulled in the latest master to make sure that it works with latest master. |
|
Thanks all for the review. I agree that it makes the code a bit more verbose, but this makes it also more accessible for newcomers 👍 |
|
Merging. Thanks for adding this, @Fokko! |
4 participants
To enable type-checking we would like to enable mypy onto the python code :)