'DeclarativeMeta' is not defined

[sqlalchemy]

'DeclarativeMeta' is not defined

[CaselIT]

I think it's currently required for how the plugin works. In any case this issue seems different from this one, so if you could open a new issue it would be great

[blokje]

Some further testing shows that this error was introduced since 1.4.3 as running the last example above with 1.4.2 shows the following errors.

# mypy --config mypy.ini --strict simple-test.py
simple-test.py:64: error: Incompatible types in assignment (expression has type "RelationshipProperty[<nothing>]", variable has type "Mapped[List[Address]]")
simple-test.py:70: error: Incompatible types in assignment (expression has type "Column[<nothing>]", variable has type "int")
simple-test.py:72: error: Incompatible types in assignment (expression has type "RelationshipProperty[<nothing>]", variable has type "Mapped[User]")
Found 3 errors in 1 file (checked 1 source file)

[zzzeek]

hi -

to the poster: I don't have a fix for this and it remains to be seen if one is possible without changing how the plugin works. this will workaround for now:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from sqlalchemy.orm.decl_api import DeclarativeMeta

for mypy people @bryanforbes @ilevkivskyi:

I have no idea why this error is being shown, even if I explicitly add the name to the symbol table, it still shows it as not found:

    api.add_symbol_table_node(
        "DeclarativeMeta", SymbolTableNode(GDEF, declarative_meta_typeinfo)
    )

what's the purpose of add_symbol_table_node() with "global definition" if mypy still complains the name isn't present? need to make it as though this name DeclartiveMeta was imported in the target file, just need to know how to do that, or if this is not possible, that would explain why the current mypy plugin doesn't try to create the metaclass? There are some behaviors that I get for free by considering these types to be with the metaclass , i can try going more the route of the older plugin using just a "this is a declarative base" flag, but it seems a plugin should be able to add new symbols to the globals of a module.

[zzzeek]

failing test in https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2682

[zzzeek]

Some further testing shows that this error was introduced since 1.4.3 as running the last example above with 1.4.2 shows the following errors.

# mypy --config mypy.ini --strict simple-test.py
simple-test.py:64: error: Incompatible types in assignment (expression has type "RelationshipProperty[<nothing>]", variable has type "Mapped[List[Address]]")
simple-test.py:70: error: Incompatible types in assignment (expression has type "Column[<nothing>]", variable has type "int")
simple-test.py:72: error: Incompatible types in assignment (expression has type "RelationshipProperty[<nothing>]", variable has type "Mapped[User]")
Found 3 errors in 1 file (checked 1 source file)

as_declarative wasn't supported in 1.4.2 and support was added in 1.4.3. the above test shows that the declarative class wasn't recognized at all which is also a bug that's been fixed.

[zzzeek]

OK this seems to fix it, but per mypy devs I shouldn't have to do this? is this wrong? why wouldn't a name with GDEF need to be in self.globals?

diff --git a/lib/sqlalchemy/ext/mypy/plugin.py b/lib/sqlalchemy/ext/mypy/plugin.py
index c8fbcd6a2..7e60ffa68 100644
--- a/lib/sqlalchemy/ext/mypy/plugin.py
+++ b/lib/sqlalchemy/ext/mypy/plugin.py
@@ -201,6 +201,10 @@ def _make_declarative_meta(
     info = target_cls.info
     info.declared_metaclass = info.metaclass_type = declarative_meta_instance
 
+    sym = SymbolTableNode(GDEF, declarative_meta_typeinfo)
+    api.add_symbol_table_node("DeclarativeMeta", sym)
+    api.globals["DeclarativeMeta"] = sym
+
 
 def _base_cls_decorator_hook(ctx: ClassDefContext) -> None:
 

[blokje]

@zzzeek

I did some more testing, and don't know if it is related. But in our code base we use Mixins to add parent columns to several tables. It is a pattern which we use for a while now and keeps our code clean. But doing this seems to generate another mypy error.

models/user.py: error: Name '_sa_Mapped' is not defined

This error is fixable by removing the addresses property from User. So there may be something going wrong with forward references?

I have not been able to find a workaround, but reproducing code is as follows:

# models/__init__.py
from typing import TYPE_CHECKING

from sqlalchemy import Column, Integer
from sqlalchemy.orm import Mapped, as_declarative, declared_attr

if TYPE_CHECKING:
    from sqlalchemy.orm.decl_api import DeclarativeMeta

@as_declarative()
class Base(object):
    @declared_attr
    def __tablename__(self) -> Mapped[str]:
        return self.__name__.lower()

    id = Column(Integer, primary_key=True)

from .user import User
from .address import Address

__all__ = ['User', 'Address']

# models/user.py
from typing import List, TYPE_CHECKING
from sqlalchemy import Column, Integer, String, ForeignKey
from sqlalchemy.orm import Mapped, relationship
from sqlalchemy.orm.decl_api import declared_attr
from sqlalchemy.orm.relationships import RelationshipProperty
from . import Base

if TYPE_CHECKING:
    from .address import Address

class User(Base):
    name = Column(String)

    addresses: Mapped[List["Address"]] = relationship("Address", back_populates="user")

class HasUser:
    @declared_attr
    def user_id(self) -> "Column[Integer]":
        return Column(
            Integer,
            ForeignKey(User.id, ondelete="CASCADE", onupdate="CASCADE"),
            nullable=False,
        )

    @declared_attr
    def user(self) -> RelationshipProperty[User]:
        return relationship(User)

# models/address.py
from typing import TYPE_CHECKING

from . import Base
from .user import HasUser

if TYPE_CHECKING:
    from .user import User
    from sqlalchemy import Integer, Column
    from sqlalchemy.orm import RelationshipProperty

class Address(Base, HasUser):
    pass

Also, if we don't have the TYPE_CHECKING block in address.py the following errors show.

models/address.py:17: error: [SQLAlchemy Mypy plugin] Can't infer type from @declared_attr on function 'user_id';  please specify a return type from this function that is one of: Mapped[<python type>], relationship[<target class>], Column[<TypeEngine>], MapperProperty[<python type>]
models/address.py:18: error: Name 'Column' is not defined
models/address.py:25: error: [SQLAlchemy Mypy plugin] Can't infer type from @declared_attr on function 'user';  please specify a return type from this function that is one of: Mapped[<python type>], relationship[<target class>], Column[<TypeEngine>], MapperProperty[<python type>]
models/address.py:26: error: Name 'RelationshipProperty' is not defined

Unfortunately I'm not really familiar with either mypy and the mypy plugin system, otherwise I could deepdive a bit more. But it feels that things are not "re-exported" correctly. Please let me know if there is a good starting point in helping out, I absolutely love typing in Python and would love to see it working for SqlAlchemy too.

If this was supposed to be opened as a new issue, my apologies, but it seemed somehow related.

[zzzeek]

@zzzeek

I did some more testing, and don't know if it is related. But in our code base we use Mixins to add parent columns to several tables. It is a pattern which we use for a while now and keeps our code clean. But doing this seems to generate another mypy error.

models/user.py: error: Name '_sa_Mapped' is not defined

This error is fixable by removing the addresses property from User. So there may be something going wrong with forward references?

yes it's likely. mypy's behavior is extremely difficult to predict as it arrives at the same result through many different codepaths depending on how the files are laid out, what was already cached in .mypy_cache or not, incremental mode, etc. We have a similar set of fixes for mypy crashes in this regard coming through in https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2680 .

I have not been able to find a workaround, but reproducing code is as follows:

OK great, this will be added to what I'm doing in the above review as I need to greatly expand the test suite to accommodate for multi-file setups as well as rerunning files for incremental mode tests.

class HasUser:
@declared_attr
def user_id(self) -> "Column[Integer]":
return Column(
Integer,
ForeignKey(User.id, ondelete="CASCADE", onupdate="CASCADE"),
nullable=False,
)

wrt mixins, my current thinking is that the mypy plugin will need a hint for mixins for cases where they are by themselves in a file and arent recognized as part of a mapped hierarchy. once i get that in with some tests it will probably look like:

from sqlalchemy.orm import declarative_mixin

@declarative_mixin
class HasUser:
   # ...

Unfortunately I'm not really familiar with either mypy and the mypy plugin system, otherwise I could deepdive a bit more. But it feels that things are not "re-exported" correctly. Please let me know if there is a good starting point in helping out, I absolutely love typing in Python and would love to see it working for SqlAlchemy too.

I just learned how to do mypy plugins in the past month and at the moment I can't point people towards an "easy" way to do it, you start out by reading the source code to the example plugins at https://github.com/python/mypy/tree/master/mypy/plugins , and then basically experimenting with print statements and pdb. I spend a lot of time inside of mypy's source finding out how things work, but it's a very large and interconnected system with multiple passes and phases that I only have a vague idea how they work. getting the plugin right now to work as well as it does (which is barely) was one of the more difficult programming tasks I've done in some years.

If this was supposed to be opened as a new issue, my apologies, but it seemed somehow related.

it's all good because this is all super -alpha stuff and we're still just getting the baseline functionality into the thing.

[bryanforbes]

OK this seems to fix it, but per mypy devs I shouldn't have to do this?

You won't have to do this for Base = declarative_base() because that happens in the "scope" of the module GDEF, but for class decorators, you're now in the "class scope", so add_symbol_table_node() is adding to ClassDef.info.names.

is this wrong?

Calling add_symbol_table_node() for class decorators adding a module-level symbol is wrong, yes. It's not wrong for declarative_base(), though.

why wouldn't a name with GDEF need to be in self.globals?

To be honest, I'm not really sure. I didn't design the API and find this part of the semantic analyzer API fairly confusing.

[bryanforbes]

wrt mixins, my current thinking is that the mypy plugin will need a hint for mixins for cases where they are by themselves in a file and arent recognized as part of a mapped hierarchy

I think this is going to have to be the case unless we want to scan all classes for declarative properties (I'd rather not do that). There may be an advantage to using a decorator, though: I don't think the plugin will need to build the properties while scanning the class hierarchies for declarative information since it will already be built and serialized into _sa_decl_class_applied by the time mypy gets to it for a child class.

[bryanforbes]

I did some digging into the plugin system and it looks like get_customize_class_mro_hook() would keep us out of the class scope:

diff --git a/lib/sqlalchemy/ext/mypy/plugin.py b/lib/sqlalchemy/ext/mypy/plugin.py
index c8fbcd6a2..f35aa08e6 100644
--- a/lib/sqlalchemy/ext/mypy/plugin.py
+++ b/lib/sqlalchemy/ext/mypy/plugin.py
@@ -119,6 +119,18 @@ def _queryable_getattr_hook(ctx: AttributeContext) -> Type:
 
 def _fill_in_decorators(ctx: ClassDefContext) -> None:
     for decorator in ctx.cls.decorators:
+        if (
+            isinstance(decorator, nodes.CallExpr)
+            and isinstance(decorator.callee, nodes.NameExpr)
+            and decorator.callee.name == "as_declarative"
+        ):
+            declarative_meta_sym: SymbolTableNode = ctx.api.modules[
+                "sqlalchemy.orm.decl_api"
+            ].names["DeclarativeMeta"]
+            declarative_meta_typeinfo: TypeInfo = declarative_meta_sym.node
+            sym = SymbolTableNode(GDEF, declarative_meta_typeinfo)
+            ctx.api.add_symbol_table_node("__sa_DeclarativeMeta", sym)
+            continue
         # set the ".fullname" attribute of a class decorator
         # that is a MemberExpr.   This causes the logic in
         # semanal.py->apply_class_plugin_hooks to invoke the
@@ -189,7 +201,7 @@ def _make_declarative_meta(
     ].names["DeclarativeMeta"]
     declarative_meta_typeinfo: TypeInfo = declarative_meta_sym.node
 
-    declarative_meta_name: NameExpr = NameExpr("DeclarativeMeta")
+    declarative_meta_name: NameExpr = NameExpr("__sa_DeclarativeMeta")
     declarative_meta_name.kind = GDEF
     declarative_meta_name.fullname = "sqlalchemy.orm.decl_api.DeclarativeMeta"
     declarative_meta_name.node = declarative_meta_typeinfo

[zzzeek]

it seems really simple and tempting to just put the names in api.globals whenever we want and be done with it. we're a plugin, we should be able to say "no really, just have this name around for us".

that said is the idea that when we call add_symbol_table_node within the mro hook, we happen to be in the right scope such that the names get put into api.globals (or somewhere else ?) in any case?

[bryanforbes]

it seems really simple and tempting to just put the names in api.globals whenever we want and be done with it. we're a plugin, we should be able to say "no really, just have this name around for us".

I totally understand, but it's not a public API, so at some point globals could be renamed to _globals and then our plugin will break

that said is the idea that when we call add_symbol_table_node within the mro hook, we happen to be in the right scope such that the names get put into api.globals (or somewhere else ?) in any case?

That's exactly right. In add_symbol_table_node(), it gets the "current" symbol table at the top and adds things to it. In the routine to get the current symbol table, it returns the local (function) scope if it is set, then it will return the class scope if the semantic analyzer's class type info property is set, and then finally if neither of those is set it will return the global scope. The order of class hooks with their scope is as follows (this is based on the code in mypy/semanal.py and me walking through it several times over the last few days, so I may have missed an edge case):

1. get_dynamic_class_hook (global)
2. get_customize_class_mro_hook (global for top-level classes, containing class for nested)
3. get_class_decorator_hook (class)
4. get_metaclass_hook (class)
5. get_base_class_hook (class)

[zzzeek]

we can go with this adjustment, but for future plugin authors I would propose mypy needs to have dedicated API to add global symbols as this is a common need for the kinds of things "plugins" do. it's really not reasonable that writing a mypy plugin needs to depend on this level of undocumented internals, I know it's a very hard problem in this case but this is quite poor support for third parties.

[bryanforbes]

but for future plugin authors I would propose mypy needs to have dedicated API to add global symbols as this is a common need for the kinds of things "plugins" do

I agree. I wonder if @JukkaL, @hauntsaninja, or @ilevkivskyi could comment on what it would take to get an API like that into the mypy plugin API.

[zzzeek]

I've got fixes for these names in sqlalchemy/sqlalchemy@1c8e221 .

[xncbf]

I'm using sqlalchemy==1.4.28 but still no fix.
If i use declared_attr in the upper mixin class, i get Name "Carrier" is not defined error.
Using import , which is not used anywhere, fixes the error, but this is not the way I want it to be.

My code in mixin.py

@declarative_mixin
class DeliveryRefundPolicyMixin:
    @declared_attr
    def default_carrier(cls) -> Mapped["Carrier"]:
        return relationship("Carrier", primaryjoin="Carrier.id==%s.default_carrier_id" % cls.__name__)

models.py

class ProductBase(AbstractBase, DeliveryRefundPolicyMixin):
    __abstract__ = True

models.py not using Carrier anywhere but got not defined error

[CaselIT]

If i use declared_attr in the upper mixin class, i get Name "Carrier" is not defined error.

where are you getting the error? in mixin.py or in models.py?

[xncbf]

If i use declared_attr in the upper mixin class, i get Name "Carrier" is not defined error.

where are you getting the error? in mixin.py or in models.py?

In models.py

[CaselIT]

the issue seems different

I think the solution here is to use TYPE_CHECKING and import Carrier there.

if TYPE_CHECKING:
  from module.to.carrier import Carrier

[xncbf]

Yes, that's right. That way the error goes away. But the Carrier is not used in model.py so it conflicts with libraries like flake8. Is this normal?

[xncbf]

@CaselIT Thanks, i just opened new issue.