-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add support for running pyanalyze as a Pre-Commit hook #216
Comments
Happy to take this! Similarly there should be a GitHub Action but I haven't gotten around to working on that. |
Thanks! Added the hook and working on testing and documenting it. One question—what version do you plan to be the next release that will include this, so I can list the correct tag in the snippit documenting this in the README? Also, once this goes live in a tagged version, it can be added to the hooks list on pre-commit.com so others can find it more easily. As for a Github Action, that would be useful as well, though for users running it through pre-commit it will be run automatically with their existing pre-commit action, or through pre-commit.ci. |
The next one should be 0.3.0. I imagine I'm going to just go with |
After spending waaaay too much time playing around with this, since design pre-commit installs its hooks in an isolated env rather than mucking with the user's current one, and it seems that pyanalyze requires not only the project's own deps to be installed, but the local project itself (which isn't possible with So I guess this should be closed, unless you have any ideas? The big blocker is that I get ImportErrors with anything that imports my local package, since it isn't installed in pre-commit's env and AFAIK by design, there's no way to get pre-commit to do so in editable mode. Maybe I could get around that with relative imports, but I'd have to change all my code just for this and absolute imports are officially recommended over relative by PEP 8. |
Thanks for looking into this! Yes, pyanalyze relies on actually importing the code it checks. Some sys.path hackery may make the pre-commit hook work, but I haven't tried myself. One of the items on my to-do list is to make it easier to run pyanalyze on arbitrary projects without too much sys.path hackery. |
Hello there @JelleZijlstra are there any plans to make the pre-commit for pyanalyze or has there been a decision made to not pursue this any further ? |
I haven't looked at this recently. I can give it a try again at some point, but I'm not sure the design of pre-commit would interact well with pyanalyze. |
This seems to work for me (if pyanalyze is installed in PATH): .pre-commit-config.yaml
pyproject.toml
|
Thanks, I got that to work on https://github.com/JelleZijlstra/taxonomy, but found that I also have to list all the project's dependencies in |
I would maybe try installing your project as an editable dependency of itself. This is done automatically with PDM running Adding it manually would be something like Currently, I am using PDM with PEP-582 and pyanalyze seems to find the dependencies in my pypackages directory just fine when running pre-commit. |
This is pretty fundamental to the design of pre-commit; its primarily intended to be used with static analysis tools that don't require access to the runtime environment. However, it can be made to work with these use cases by either each user manually specifying their requisite If users add |
As I'm sure you're aware, Pre-Commit is a popular Python/cross-language hooks framework for managing variety of checking, linting, formatting and other tools automatically, on demand in in CIs. Adding a minimal
.pre-commit-hooks.yaml
file in the root of the repo with a few lines of config will allow other projects to easily adopt pyanalyze, either locally or on CI, by simply pasting a couple-line snippet into their.pre-commit-config.yaml
file, and pre-commit will install, run and update pyanalyze automagically, making it much easier to adopt, configure and maintain.I'd be happy to add this in a PR if you don't object; I'd also document it with the standard snippit in the README. Once this goes in (as well as PR #215 ), it would be nice to have a release sometime soon so we can start taking advantage of it. Thanks!
The text was updated successfully, but these errors were encountered: