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

pre-commit mypy additional dependencies when pre-commit-config.yaml is inside child directory #174

Closed
JitPackJoyride opened this issue Sep 20, 2020 · 11 comments

Comments

@JitPackJoyride
Copy link

- repo: https://github.com/pre-commit/mirrors-mypy
   rev: v0.782
   hooks:
     - id: mypy
       args: [--strict-optional, --ignore-missing-imports, --follow-imports=skip]
       additional_dependencies:
         - pydantic
         - sqlalchemy-stubs

Currently my .pre-commit-config.yaml looks like the above. However, adding sqlalchemy-stubs to the additional_dependencies doesn't do anything to change the behaviour. Adding sqlmypy to setup.cfg works just fine however.

If someone on the team or @asottile can help, that'd be much appreciated.

@asottile
Copy link

seems to work fine for me?

$ cat t.py 
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import Column, Integer, String

Base = declarative_base()

class User(Base):
    __tablename__ = 'users'
    id = Column(Integer, primary_key=True)
    name = Column(String)

user = User(id=42, name='foo')
reveal_type(user.id)
$ cat .pre-commit-config.yaml 
repos:
-   repo: https://github.com/pre-commit/mirrors-mypy
    rev: v0.782
    hooks:
    -   id: mypy
$ pre-commit run --all-files
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

t.py:6: error: Variable "t.Base" is not valid as a type
t.py:6: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
t.py:6: error: Invalid base class "Base"
t.py:12: note: Revealed type is 'Any'
Found 2 errors in 1 file (checked 1 source file)

$ echo $'[mypy]\nplugins = sqlmypy' > setup.cfg
$ echo '        additional_dependencies: [sqlalchemy-stubs]' >> .pre-commit-config.yaml 
$ pre-commit run --all-files
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

t.py:12: note: Revealed type is 'builtins.int*'

note that pre-commit doesn't do anything special, it's just calling mypy filename filename filename

@JitPackJoyride
Copy link
Author

JitPackJoyride commented Sep 20, 2020

When running mypy independently, I get the following:

$ mypy --ignore-missing-imports --follow-imports=skip --strict-optional .
Success: no issues found in 77 source files

However, when I run pre-commit run --all-files, I get this:

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

api/tests/unit/conftest.py:27: error: "Type[Base]" has no attribute "metadata"
api/tests/unit/conftest.py:29: error: "Type[Base]" has no attribute "metadata"
api/alembic/env.py:21: error: "Type[Base]" has no attribute "metadata"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/src/crud/crud_user.py:22: error: Unexpected keyword argument "{field}" for "{table_class}"
api/tests/integration/conftest.py:47: error: "Type[Base]" has no attribute "metadata"
api/tests/integration/conftest.py:50: error: "Type[Base]" has no attribute "metadata"
Found 11 errors in 4 files (checked 41 source files)

Posting this first to give some context - but I'll continue trying to debug as well.

@asottile
Copy link

this is ~roughly what pre-commit would do:

virtualenv venv
venv/bin/pip install mypy sqlalchemy-stubs
venv/bin/mypy ... $(git ls-files -- '*.py')

also a general tip for open source, don't post images of output or code, they are inaccessible (can't be searched, can't be viewed in the viewers preferred contrast/accessibility settings, etc.) -- post text of text

@JitPackJoyride
Copy link
Author

JitPackJoyride commented Sep 20, 2020

Thanks for the tip to post text only.

I reproduced the problem and found that my error only happens specifically when my entire python project (including .pre-commit-config.yaml) is in a child directory that's not where I ran git init. Not sure how to proceed from here.

@asottile
Copy link

you could try sharing a reproducible example, so fare we're just guessing at your problem

@JitPackJoyride
Copy link
Author

https://repl.it/@AjitZK/directory-broken-mypy

^ Here's an accessible example

@asottile
Copy link

can you put a repo on github, I'm not sure what to do with repl it

@JitPackJoyride
Copy link
Author

@asottile
Copy link

and translating what I told you above, here's a reproduction without pre-commit:

$ rm -rf venv && virtualenv venv >& /dev/null && venv/bin/pip install mypy sqlalchemy-stubs >& /dev/null && venv/bin/mypy --ignore-missing-imports --scripts-are-modules $(git ls-files -- '*.py')
backend/alembic/env.py:3: error: "Type[Base]" has no attribute "metadata"
Found 1 error in 1 file (checked 9 source files)

an aside, pre-commit's configuration is conventionally at the root of the repository and git hooks execute from the root of the repository

@asottile
Copy link

telling mypy where your configuration is seems to work though:

$ venv/bin/mypy --config backend/setup.cfg --ignore-missing-imports --scripts-are-modules $(git ls-files -- '*.py')
Success: no issues found in 9 source files

@JitPackJoyride
Copy link
Author

Ok, then I'm changing my .pre-commit-config.yaml to have:

- repo: https://github.com/pre-commit/mirrors-mypy
   rev: v0.782
   hooks:
     - id: mypy
       args: [--config=backend/setup.cfg, --strict-optional, --ignore-missing-imports, --follow-imports=skip]
       additional_dependencies:
         - pydantic
         - sqlalchemy-stubs

This fixes my issue. Will need to refactor to properly set up pre-commit to take effect from the root of the repository in the future.

@JitPackJoyride JitPackJoyride changed the title Support for pre-commit mypy additional dependencies pre-commit mypy additional dependencies when pre-commit-config.yaml is inside child directory Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants