dropbox / sqlalchemy-stubs

Mypy plugin and stubs for SQLAlchemy
Apache License 2.0
570 stars 101 forks source link

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

Closed JitPackJoyride closed 3 years ago

JitPackJoyride commented 3 years ago
- 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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

JitPackJoyride commented 3 years ago

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

^ Here's an accessible example

asottile commented 3 years ago

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

JitPackJoyride commented 3 years ago

https://github.com/AjitZK/directory-broken-mypy

^ as a repo

asottile commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.