-
Notifications
You must be signed in to change notification settings - Fork 125
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
fix: mypy update #606
fix: mypy update #606
Conversation
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
I'm not a fan of specifying dependencies in |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
I've reverted the env changes for now, so the CI fix can go though I don't like the current setup:
The example you gave doesn't work today, we don't provide an I'd normally put the type check in pre-commit for any smaller package, but the drawbacks are the same (though bumping mypy would be easier). Honestly, the best way to do this today that would support all possible ways to run it, like in editors, would probably be a requirements.txt. And I still don't know if you'd put mypy in it - several editors have their own type checkers. I don't feel very strongly about it, though, so as long as we can keep things up to date and working, I'm fine if it can't be ideal IMO. |
I'm not sure I agree. For development, probably not. But before publishing, I feel you should type check the built package, just as you would test against the built package.
We can do with self-referencing extras. But
Maybe I should write a tool to parse pre-commit dependencies and install them in the current env with pip, for troglodytes comme moi to use. |
This fixes the issue with two type ignores no longer being needed, updates mypy, moves the configuration out of extras (it was outdated and not helpful to force pin mypy in extras, and not required to use mypy to type anyway, and removes the requirement that build be installed to type check (which is not needed).