fix(typing): allow DeclarativeBase subclasses in with_for_update(of=...) #12731
Conversation
This commit updates the `_ForUpdateOfArgument` type alias used in the `with_for_update(of=...)` method to explicitly allow `Type[DeclarativeBase]` subclasses. In SQLAlchemy 2.x, `DeclarativeBase` is the recommended way to define ORM models. While runtime behavior already accepts these classes as valid arguments for the `of` parameter, static type checkers like mypy and pyright currently report type errors when `DeclarativeBase` subclasses are passed. This patch resolves issue sqlalchemy#12730 by expanding `_ForUpdateOfArgument` to include `Type[DeclarativeBase]` (and sequences containing them), aligning type hints with actual valid runtime behavior. Fixes sqlalchemy#12730.
zzzeek
left a comment
zzzeek
left a comment
There was a problem hiding this comment.
hey there-
thanks for this! I've tested locally and this just needs _FromClauseArgument:
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 349f18930..f1959db43 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -177,7 +177,7 @@ _ForUpdateOfArgument = Union[
"_FromClauseArgument",
],
# or sequence of single column elements
- Sequence["_ColumnExpressionArgument[Any]"],
+ Sequence[Union["_ColumnExpressionArgument[Any]","_FromClauseArgument"]],
]
mypy passes your original test in that case.
if you can also add a test case right in test/typing/plain_files/sql/common_sql_element.py you'll see plenty of other cases for "of" pointing to ORM entities
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| from sqlalchemy.orm import DeclarativeBase |
There was a problem hiding this comment.
this isn't needed (also we can't have ORM imports in core in any case, DeclarativeBase is too narrow as well)
| Union[ | ||
| "_ColumnExpressionArgument[Any]", | ||
| "_FromClauseArgument", | ||
| Type[DeclarativeBase], |
There was a problem hiding this comment.
this is not needed, _FromClauseArgument accommodates mapped classes
| ], | ||
| # or sequence of single column elements | ||
| Sequence["_ColumnExpressionArgument[Any]"], | ||
| Sequence[Union["_ColumnExpressionArgument[Any]", Type[DeclarativeBase]]], |
There was a problem hiding this comment.
this list needs _FromClauseArgument if the "of" can point to lists of tables, which will accommodate the ORM mapped classes as well
2 participants
This commit updates the
_ForUpdateOfArgumenttype alias used in thewith_for_update(of=...)method to explicitly allowType[DeclarativeBase]subclasses.In SQLAlchemy 2.x,
DeclarativeBaseis the recommended way to define ORM models. While runtime behavior already accepts these classes as valid arguments for theofparameter, static type checkers like mypy and pyright currently report type errors whenDeclarativeBasesubclasses are passed.This patch resolves issue #12730 by expanding
_ForUpdateOfArgumentto includeType[DeclarativeBase](and sequences containing them), aligning type hints with actual valid runtime behavior.Fixes #12730.