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

Metadata naming_convention can't be of type dict[str, str] in v2.0.20 #10264

Closed
2 tasks done
HassanAbouelela opened this issue Aug 21, 2023 · 11 comments
Closed
2 tasks done
Labels
bug Something isn't working near-term release addition to the milestone which indicates this should be in a near-term release regression something worked and was broken by a change typing pep -484 typing issues. independent of "mypy"
Milestone

Comments

@HassanAbouelela
Copy link

Ensure stubs packages are not installed

  • No sqlalchemy stub packages is installed (both sqlalchemy-stubs and sqlalchemy2-stubs are not compatible with v2)

Verify if the api is typed

  • The api is not in a module listed in #6810 so it should pass type checking

Describe the typing issue

In SQLAlchemy 2.0.20, the type of ORM Metadata's naming_convention was updated from Optional[dict[str, str]] to

_NamingSchemaParameter = Union[
    _NamingSchemaTD,
    Mapping[
        Union[Type[_AllConstraints], str], Union[str, _NamingSchemaCallable]
    ],
]

Mypy now will not accept a dict[str, str] as seen in doc examples, with the error below.

To Reproduce

from sqlalchemy.orm import DeclarativeBase
from sqlalchemy.schema import MetaData

NAMING_CONVENTIONS = {
    "ix": "%(column_0_label)s_ix",
    "uq": "%(table_name)s_%(column_0_name)s_uq",
    "ck": "%(table_name)s_%(constraint_name)s_ck",
    "fk": "%(table_name)s_%(column_0_name)s_%(referred_table_name)s_fk",
    "pk": "%(table_name)s_pk"
}


class Base(DeclarativeBase):
    metadata = MetaData(naming_convention=NAMING_CONVENTIONS)

Error

error: Argument "naming_convention" to "MetaData" has incompatible type "dict[str, str]"; expected "_NamingSchemaTD | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | None"  [arg-type]

Versions

  • OS: Windows
  • Python: 3.11
  • SQLAlchemy: 2.0.20
  • Type checker (eg: mypy 0.991, pyright 1.1.290, etc): mypy 1.5.1

Additional context

I validated the offending code in 2.0.19, with the same mypy configuration and did not encounter an issue.

I tried modifying the types in the source code, and it is introduced by the Type[_AllConstraints]. Removing that portion of the type resolves the issue. I did not investigate further, and this might be a mypy bug.

If relevant, here is the mypy configuration from my pyproject.toml:

[tool.mypy]
plugins = "pydantic.mypy"
strict = true
implicit_reexport = true
show_error_codes = true
show_error_code_links = true
enable_error_code = "ignore-without-code"
@HassanAbouelela HassanAbouelela added requires triage New issue that requires categorization typing pep -484 typing issues. independent of "mypy" labels Aug 21, 2023
@CaselIT
Copy link
Member

CaselIT commented Aug 21, 2023

Hi,

That seems a mypy bug, since the type allows for a dict[str,str]. I will try unioning the existing type with a Mapping[str,str] to see if mypy understands that

@CaselIT CaselIT added bug Something isn't working and removed requires triage New issue that requires categorization labels Aug 21, 2023
@eltoder
Copy link
Contributor

eltoder commented Aug 21, 2023

@CaselIT This is not a mypy bug: Mapping type is covariant in values, but not in keys. You can see python/typing#273 and python/typing#445 for some discussion.
So, for example, Mapping[str, str | int] is a super type of Mapping[str, str], Mapping[str | int, str] is not. You may have to rewrite the type be a union of Mapping instead of a Mapping with a union of keys.

@CaselIT
Copy link
Member

CaselIT commented Aug 21, 2023

strange that pyright likes the current type, while mypy does not. Usually pyright is the correct one on complex types

@CaselIT
Copy link
Member

CaselIT commented Aug 21, 2023

strange that pyright likes the current type, while mypy does not. Usually pyright is the correct one on complex types

actually it seems that it's also erroring

@CaselIT
Copy link
Member

CaselIT commented Aug 21, 2023

I'm not sure how to proceed. The typing for mapping seem too constrained to allow the actual type of this attribute.
I think this type is correct

_AllConstraints = Union[
    Index,
    UniqueConstraint,
    CheckConstraint,
    ForeignKeyConstraint,
    PrimaryKeyConstraint,
]
_NamingSchemaCallable = Callable[[_AllConstraints, Table], str]
_NamingSchemaValue = Union[str, _NamingSchemaCallable]


class _NamingSchemaTD(TypedDict, total=False):
    fk: _NamingSchemaValue
    pk: _NamingSchemaValue
    ix: _NamingSchemaValue
    ck: _NamingSchemaValue
    uq: _NamingSchemaValue


_NamingSchemaParameter = Union[
    _NamingSchemaTD,
    Mapping[str, _NamingSchemaValue],
    Mapping[Type[_AllConstraints], _NamingSchemaValue],
    Mapping[Union[Type[_AllConstraints], str], _NamingSchemaValue],
]

but it's still reporting errors on

from typing import Union

from sqlalchemy import Constraint
from sqlalchemy import Index
from sqlalchemy import MetaData
from sqlalchemy import Table
from sqlalchemy import UniqueConstraint

MetaData(
    naming_convention={
        "ix": "ix_%(column_0_label)s",
        "uq": "uq_%(table_name)s_%(column_0_name)s",
        "ck": "ck_%(table_name)s_%(constraint_name)s",
        "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
        "pk": "pk_%(table_name)s",
    }
)


MetaData(
    naming_convention={
        Index: "ix_%(table_name)s",
        "uq": "uq_%(table_name)s_%(column_0_N_name)s",
    }
)


def fk_guid(constraint: Union[Constraint, Index], table: Table) -> str:
    return "foo"


MetaData(
    naming_convention={
        "fk_guid": fk_guid,
        "ix": "ix_%(column_0_label)s",
        "fk": "fk_%(fk_guid)s",
        "foo": lambda c, t: t.name + str(c.name),
    }
)

naming_convention = {
    "ix": "ix_%(column_0_label)s",
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(constraint_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s",
}
MetaData(naming_convention=naming_convention)

naming_convention2 = {
    Index: "ix_%(column_0_label)s",
    UniqueConstraint: "uq_%(table_name)s_%(column_0_name)s",
}
MetaData(naming_convention=naming_convention2)

naming_convention3 = {
    Index: "ix_%(column_0_label)s",
    "ck": "ck_%(table_name)s_%(constraint_name)s",
}
MetaData(naming_convention=naming_convention3)

mypy

test\typing\plain_files\sql\schema.py:21: error: Argument "naming_convention" to "MetaData" has incompatible type "dict[object, str]"; expected "_NamingSchemaTD | Mapping[str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint], str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | None"  [arg-type]
test\typing\plain_files\sql\schema.py:22: error: Expected TypedDict key to be string literal  [misc]
test\typing\plain_files\sql\schema.py:33: error: Argument "naming_convention" to "MetaData" has incompatible type "dict[str, object]"; expected "_NamingSchemaTD | Mapping[str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint], str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | None"  [arg-type]
test\typing\plain_files\sql\schema.py:60: error: Argument "naming_convention" to "MetaData" has incompatible type "dict[object, str]"; expected "_NamingSchemaTD | Mapping[str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint], str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, str | Callable[[Index | UniqueConstraint | CheckConstraint | ForeignKeyConstraint | PrimaryKeyConstraint, Table], str]] | None"  [arg-type]
Found 4 errors in 1 file (checked 266 source files)

while pyright reports

test\typing\plain_files\sql\schema.py
  test\typing\plain_files\sql\schema.py:54:28 - error: Argument of type "dict[type[Index] | type[UniqueConstraint], str]" cannot be assigned to parameter "naming_convention" of type "_NamingSchemaParameter | None" in function "__init__"
    Type "dict[type[Index] | type[UniqueConstraint], str]" cannot be assigned to type "_NamingSchemaParameter | None"
      "dict[type[Index] | type[UniqueConstraint], str]" is incompatible with "_NamingSchemaTD"
      "dict[type[Index] | type[UniqueConstraint], str]" is incompatible with "Mapping[str, _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | type[UniqueConstraint]" is not the same as "str"
      "dict[type[Index] | type[UniqueConstraint], str]" is incompatible with "Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint], _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | type[UniqueConstraint]" is not the same as "type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint]"
      "dict[type[Index] | type[UniqueConstraint], str]" is incompatible with "Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | type[UniqueConstraint]" is not the same as "type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str"
    ... (reportGeneralTypeIssues)
  test\typing\plain_files\sql\schema.py:60:28 - error: Argument of type "dict[type[Index] | str, str]" cannot be assigned to parameter "naming_convention" of type "_NamingSchemaParameter | None" in function "__init__"
    Type "dict[type[Index] | str, str]" cannot be assigned to type "_NamingSchemaParameter | None"
      "dict[type[Index] | str, str]" is incompatible with "_NamingSchemaTD"
      "dict[type[Index] | str, str]" is incompatible with "Mapping[str, _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | str" is not the same as "str"
      "dict[type[Index] | str, str]" is incompatible with "Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint], _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | str" is not the same as "type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint]"
      "dict[type[Index] | str, str]" is incompatible with "Mapping[type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str, _NamingSchemaValue]"
        Type parameter "_KT@Mapping" is invariant, but "type[Index] | str" is not the same as "type[Index] | type[UniqueConstraint] | type[CheckConstraint] | type[ForeignKeyConstraint] | type[PrimaryKeyConstraint] | str"
    ... (reportGeneralTypeIssues)
2 errors, 0 warnings, 0 informations

suggestions that are not Mapping[Any, _NamingSchemaValue]?

@zzzeek
Copy link
Member

zzzeek commented Aug 21, 2023

seems so. Not sure if this is a full revert or not but we have to go with

diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 008ae2c005..8d97254490 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -5297,21 +5297,7 @@ _AllConstraints = Union[
 _NamingSchemaCallable = Callable[[_AllConstraints, Table], str]
 
 
-class _NamingSchemaTD(TypedDict, total=False):
-    fk: Union[str, _NamingSchemaCallable]
-    pk: Union[str, _NamingSchemaCallable]
-    ix: Union[str, _NamingSchemaCallable]
-    ck: Union[str, _NamingSchemaCallable]
-    uq: Union[str, _NamingSchemaCallable]
-
-
-_NamingSchemaParameter = Union[
-    _NamingSchemaTD,
-    Mapping[
-        Union[Type[_AllConstraints], str], Union[str, _NamingSchemaCallable]
-    ],
-]
-
+_NamingSchemaParameter = Mapping[Any, Union[str, _NamingSchemaCallable]]
 
 DEFAULT_NAMING_CONVENTION: _NamingSchemaParameter = util.immutabledict(
     {"ix": "ix_%(column_0_label)s"}  # type: ignore[arg-type]

@zzzeek zzzeek added regression something worked and was broken by a change near-term release addition to the milestone which indicates this should be in a near-term release labels Aug 21, 2023
@zzzeek zzzeek added this to the 2.0.x milestone Aug 21, 2023
@CaselIT
Copy link
Member

CaselIT commented Aug 21, 2023

that's a shame. as always python typing are quite lacking...

@BartlomiejSkwira
Copy link

My workaround to silence the mypy error (until a proper fix is released):

from sqlalchemy.sql.schema import _NamingSchemaTD

# use the convention that you have in your project
convention = _NamingSchemaTD(
    ix="ix_%(table_name)s_%(column_0_N_name)s",
    uq="uq_%(table_name)s_%(column_0_N_name)s",
    ck="ck_%(table_name)s_%(constraint_name)s",
    fk="fk_%(table_name)s_%(column_0_N_name)s_%(referred_table_name)s",
    pk="pk_%(table_name)s",
)

metadata = sqlalchemy.MetaData(naming_convention=convention)

@CaselIT
Copy link
Member

CaselIT commented Sep 12, 2023

Defining it inline also works correctly

import sqlalchemy as sa

metadata = sa.MetaData(
    naming_convention=dict(
        ix="ix_%(table_name)s_%(column_0_N_name)s",
        uq="uq_%(table_name)s_%(column_0_N_name)s",
        ck="ck_%(table_name)s_%(constraint_name)s",
        fk="fk_%(table_name)s_%(column_0_N_name)s_%(referred_table_name)s",
        pk="pk_%(table_name)s",
    )
)

@zzzeek
Copy link
Member

zzzeek commented Sep 12, 2023

we didnt even note this in the changelog for 2.0.20

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

repair typing for the ordinary case of naming_convention w/ str https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4882

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working near-term release addition to the milestone which indicates this should be in a near-term release regression something worked and was broken by a change typing pep -484 typing issues. independent of "mypy"
Projects
None yet
Development

No branches or pull requests

7 participants
@zzzeek @BartlomiejSkwira @eltoder @sqla-tester @CaselIT @HassanAbouelela and others