Introduce mypy and enforce it on a few files #40
Conversation
src/bandersnatch/configuration.py
Outdated
| from typing import Any | ||
| from typing import Dict | ||
| from typing import Optional | ||
| from typing import Type |
There was a problem hiding this comment.
Why not all one from?
i.e.
from typing import (Any, Dict, Optional, Type)
There was a problem hiding this comment.
personal preference though if you'd like a different style I will change it.
I use reorder_python_imports -- the style here aims to reduce merge conflicts above everything else.
If you'd like, I could set you up with isort in this PR as well, that way we don't have to bicker about needless style
| @@ -89,7 +94,7 @@ def update_safe(filename, **kw): | |||
| prefix=os.path.basename(filename) + '.', **kw) as tf: | |||
| if os.path.exists(filename): | |||
| os.chmod(tf.name, os.stat(filename).st_mode & 0o7777) | |||
| tf.has_changed = False | |||
| tf.has_changed = False # type: ignore | |||
There was a problem hiding this comment.
What's the error here? I feel we should be able to fix it.
There was a problem hiding this comment.
has_changed is not an attribute of file objects, there's attribute stuffing happening here
| @@ -1,7 +1,4 @@ | |||
| flake8 | |||
There was a problem hiding this comment.
Don't we still use flake8?
There was a problem hiding this comment.
yes, but via pre-commit. pre-commit manages the installation in a separate environment so you don't have to worry about system installation of tools
There was a problem hiding this comment.
I only like people to have the ability to run things manually.
- Sorry about all the questions, I am a GitHub noob, been locked away in Facebook for to long.
I'm not quite Dave Harrington n00b, but close.
There was a problem hiding this comment.
oh hey there @DaveHarrington -- small world I guess?
|
All sounds good - If you're down to add |
| pre-commit run --all-files --show-diff-on-failure | ||
|
|
||
| [isort] | ||
| include_trailing_comma = True |
There was a problem hiding this comment.
I'll fix this if black does not agree here. I can't remember.
|
I'll also plan to use MonkeyType or like tool to get more type annotations for free. |
to be honest, I'm not exactly sure how I got here -- I was reading through the project to try and understand what it does and noticed #15 and somehow my first thought was "oh hey, that sounds like fun". I applied it to a few modules and then got to a point where I needed to actually understand the code to make further progress and so I decided to commit what I had and make a PR.
This also (selfishly?) adds pre-commit as an easier way to run linting (
I noticed there wasn't currently any automated linting in CI so I added it!EDIT: had to change from pep8 -> flake8 so that type annotations weren't false positives) -- this part isn't entirely necessary as mypy can be invoked directly.