sqlalchemy / sqlalchemy2-stubs

PEP-484 typing stubs for SQLAlchemy 1.4
MIT License
159 stars 41 forks source link

Cannot define declarative base class as attribute of object #54

Open MaicoTimmerman opened 3 years ago

MaicoTimmerman commented 3 years ago

Describe the bug I'd like to switch from the dropbox/sqlalchemy stubs to the official stubs, however when the declarative base is defined as attribute of an instance, it is not recognized as valid base class.

The Dropbox stubs give similar errors regarding the subclassing, however the types are still correctly inferred.

Expected behavior Type definitions to be picked up as the builtin types: test.py:20: note: Revealed type is 'Union[builtins.int, None]'

To Reproduce

from sqlalchemy.schema import Column
from sqlalchemy.types import Integer, String
from sqlalchemy.orm.decl_api import DeclarativeMeta, declarative_base

db = DB()

class DB():
    def __init__(self):
        self.Base = declarative_base()

class A(db.Base):
    __tablename__ = "a"
    id = Column(Integer, primary_key=True)

a = A()
reveal_type(a.id)

Error

test2.py:12: error: Name 'db.Base' is not defined
test2.py:17: note: Revealed type is 'sqlalchemy.sql.schema.Column[sqlalchemy.sql.sqltypes.Integer*]'
Found 1 error in 1 file (checked 1 source file)

Versions.

Additional context The problem seems similar to one described on StackOverflow. However, I expected that replacing the direct subclassing with a typed variable would resort the problem.

Example code ```py from sqlalchemy.schema import Column from sqlalchemy.types import Integer, String from sqlalchemy.orm.decl_api import DeclarativeMeta, declarative_base db = DB() class DB(): def __init__(self): self.Base = declarative_base() Base: DeclarativeMeta = db.Base class A(Base): __tablename__ = "a" id = Column(Integer, primary_key=True) a = A() reveal_type(a.id) ```

Have a nice day!

zzzeek commented 3 years ago

your example refers to "DB" before the class is declared, which would be part of the problem. However, I can't get this pattern to work at all for Mypy, even without any dynamic classes or anything:


class Foo:
    pass

class DB:
    def __init__(self):
        self.Base = Foo

db = DB()

class A(db.Base):
    pass
$ mypy test3.py 
test3.py:13: error: Name 'db.Base' is not defined
Found 1 error in 1 file (checked 1 source file)

not sure what I'm missing? or is this a known mypy bug?

zzzeek commented 3 years ago

oh, as far as why that prevents everything else from working, the SQLAlchemy stubs rely upon the mypy plugin for declarative mappings to be supported. The plugin can't work here because mypy is not giving the plugin any information about declarative_base() here because as above Mypy seems to be broken for this case.

layday commented 3 years ago

It's generally the case that the return type of a callable can't be used in typing contexts. Mypy should probably print something more informative than "Name 'db.Base' is not defined" here.

MaicoTimmerman commented 3 years ago

I think this might fall under the unsupported python features: Run-time evaluated base classes like class A(foo()): pass, Similar to metaclasses [1]. Currently, there is support for detecting the declarative_base() function directly and replace it with a full blown type including mro, perhaps that could be expanded to also detect it in functions and classes? Like:

from sqlalchemy.schema import Column
from sqlalchemy.types import Integer
from sqlalchemy.orm.decl_api import declarative_base

def my_declarative_base():
    return declarative_base()

Base = my_declarative_base()

class A(Base):
    __tablename__ = "a"
    id = Column(Integer, primary_key=True)

a = A()
reveal_type(a.id)

[1] https://github.com/python/mypy/wiki/Unsupported-Python-Features

zzzeek commented 3 years ago

if you have a standalone Base, the mypy plugin supports that just fine through the existing plugin hooks.

Also I would recommend, if you truly have "decalrative bases per instance of X" going on, use the decorator pattern, where the "registry" is the attribute of your object that contains "base". Mypy seems to be more forgiving about this kind of thing (but maybe not in this case still, not sure)

synic commented 2 years ago

This definitely seems more like a problem with mypy, and not anything to do with the sqlalchemy-stubs:

from typing import Type

class FakeBaseModel:
    pass

class FakeDatabase:
    Base: Type[FakeBaseModel]

    def __init__(self) -> None:
        self.Base = FakeBaseModel

db = FakeDatabase()

class FakeModel(db.Base):
    pass
[mypytest {mypytest}]$ mypy .
mypytest/models.py:18: error: Name "db.Base" is not defined
Found 1 error in 1 file (checked 2 source files)

From https://stackoverflow.com/questions/71069074/appeasing-mypy-trying-to-use-an-instance-variable?noredirect=1#comment125650705_71069074

zzzeek commented 2 years ago

yes I had a convo with the typing folks and functions that return classes are a no-go in general, none of the tools support it