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

[Question] why can't we add pylint in this repo hooks.yaml file? #157

Closed
lavish205 opened this issue Dec 7, 2016 · 2 comments
Closed

[Question] why can't we add pylint in this repo hooks.yaml file? #157

lavish205 opened this issue Dec 7, 2016 · 2 comments
Labels

Comments

@lavish205
Copy link

My .pre-commit-config.yaml file looks like this:

-   repo: https://github.com/pre-commit/pre-commit-hooks
     sha: 52582865ab7ed8f41734253190844c854ddd9a9e
     hooks:
     -   id: trailing-whitespace
     -   id: check-ast
     -   id: debug-statements
     -   id: end-of-file-fixer
-   repo: git://github.com/pre-commit/mirrors-pylint
     sha: v1.6.4
     hooks:
    -   id: pylint

and when I run pre-commit install it creates a repo for pylint in ~/.pre-commit/repoXXXX/ with its own virtual env in it, due to which pylint shows import errors.
So there are two ways to solve this error:

  1. add all required packages in additional_dependencies in .pre-commit-config.yaml file for pylint.
    But I can't add all the packages here because it useless to create two virtual env with same packages.
    Can we however, merge it with our own virtualenv??

  2. if I modify pre-commit-hooks/hooks.yaml and add pylint hook over there, it works perfectly fine.

Can you suggest me which is the best way to do it??

@asottile
Copy link
Member

asottile commented Dec 7, 2016

So tl;dr is pylint does some static analysis and some dynamic analysis. When put in an isolated virtualenv it does not work properly. Using isolated virtualenvs achieves the design goal of reproducibility in pre-commit.

That said, there are still a few options for using pylint with pre-commit:

  • disable the dynamic checking parts (not usually the desirable option)
  • use it as a "system" hook (usually through "local" hooks).

You can find some related issues:

@asottile asottile closed this as completed Dec 9, 2016
0xangelo added a commit to 0xangelo/deep-rl that referenced this issue Jun 16, 2019
Was having errors similar to this issue:
`pre-commit/pre-commit-hooks#157
Using it locally fixes `import-error`.

Signed-off-by: Ângelo Lovatto <angelolovatto@gmail.com>
aleb added a commit to aleb/mirrors-pylint that referenced this issue Dec 21, 2019
As described in pre-commit/pre-commit-hooks#157
using this hook runs pylint isolated from everything else.
aleb added a commit to aleb/mirrors-pylint that referenced this issue Jan 8, 2020
As described in pre-commit/pre-commit-hooks#157
using this hook runs pylint isolated from everything else.
namanjain pushed a commit to square/bionic that referenced this issue Oct 8, 2020
Looks like pre-commit creates its own virtual environment (which shows
up in ~/.cache/pre-commit for me). This breaks with flake8 for the new
custom linter. It seems like pylint has the same problem with missing
modules as I have with flake8, with the missing modules due to different
virtualenvs. One of the recommended options from pre-commit devs (found
in [this issue](pre-commit/pre-commit-hooks#157))
is to use system language in the hooks to get around this problem. You
can find the documentation for system language [here](https://pre-commit.com/#system).
namanjain pushed a commit to square/bionic that referenced this issue Oct 8, 2020
Looks like pre-commit does some static analysis by caching packages (which shows
up in ~/.cache/pre-commit for me). This breaks with flake8 for the new
custom linter. It seems like pylint has the same problem with missing
modules as I have with flake8, with the missing modules due to different
virtualenvs. One of the recommended options from pre-commit devs (found
in [this issue](pre-commit/pre-commit-hooks#157))
is to use system language in the hooks to get around this problem. You
can find the documentation for system language [here](https://pre-commit.com/#system).
@swateek
Copy link

swateek commented Jun 22, 2021

https://github.com/Yelp/venv-update/blob/de9e1befdecf448cf02ceb5c57a30bcc88a58c06/.pre-commit-config.yaml#L30-L36

is a good example, thank you!

Also, needed to install pylint

pip3 install pylint

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

No branches or pull requests

3 participants