🐛 Fix pyright errors in type annotations and select() overloads#1820
🐛 Fix pyright errors in type annotations and select() overloads#1820estarfoo wants to merge 3 commits intofastapi:mainfrom
select() overloads#1820Conversation
Replace the `__`-prefix convention (PEP 484) with explicit `/` syntax (PEP 570) for positional-only parameters in generated `select()` overloads. Pyright enforces that `__`-prefixed (positional-only) parameters cannot appear before non-prefixed parameters in the same signature, which caused 30 errors in `_expression_select_gen.py`. Mypy does not enforce this, so it was never caught. Both the Jinja2 template and the generator script are updated so that `test_select_gen` continues to pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `sa_type` parameter on `Field()` was typed as `type[Any]` (class only), but the runtime in `get_sqlalchemy_type()` already accepts both classes and instances. Users passing instances like `BigInteger()` or `Numeric(precision=10, scale=2)` got false type errors from pyright. Widen the annotation to `type[Any] | TypeEngine[Any]` in the `FieldInfoMetadata` dataclass, all three `Field()` overloads, and the implementation. Add tests for `BigInteger()` and `Numeric(...)` instances in `test_field_sa_column.py`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `SQLModel` base class declared `__tablename__` both as a `ClassVar` and as a `@declared_attr` method. Pyright saw the descriptor type from `@declared_attr`, causing `reportAssignmentType` errors when subclasses set `__tablename__ = "my_table"` — a pattern that works at runtime. Replace the `@declared_attr` method with a default set in `SQLModelMetaclass.__new__` via `dict_used`, before class creation. This preserves the `ClassVar[str | Callable]` declaration and makes plain string assignments work without type errors. Fixes fastapi#98. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Extra notes:
|
|
As for |
Hm, out of curiousity, if you have zero experience with sqlmodel, then why contribute a PR? You said you manually reviewed everything? |
|
Thank you both very much for the lightning-fast responses! I thought I had looked through open PRs, and I'm sorry for my lack of diligence. @svlandeg, #1806 may fix my use case for @YuriiMotov, would the test code in I'll follow up with a PR addressing |
@svlandeg, I've read (parts of) the library code and documentation, and I've tried to plug sqlmodel into my application as replacement for another ORM. Beyond that: I don't know the user community's idiomatic workflows. I can only hope that my changes are either conservative enough not to break anyone else's apps. I tend to rely on IDE hints for call signatures and type inconsistencies. In my case that's VS Code, and it uses pyright under the hood. (Which has led me to pull pyright into my CI pipelines as well.) I could disable specific diagnostics, but they're exactly the ones that'd help me use the library correctly, if not for false positives. That's without any expectation that you should be using the same tools, or focus on supporting them here. Thanks again for your feedback! |
4 participants
This PR fixes three sources of pyright diagnostics for users of the library:
select()overloads: replace__-prefix positional-only convention with explicit/syntax (PEP 570, since Python 3.8), resolving pyright diagnostics in_expression_select_gen.pysa_typeannotation: widen fromtype[Any]totype[Any] | TypeEngine[Any], matchingget_sqlalchemy_type()which already accepts instances likeBigInteger()__tablename__override: move default from@declared_attrmethod to metaclass__new__, so__tablename__ = "my_table"no longer conflicts with the descriptor type. Fixes Pyright cannot recognize the type ofSQLModel.__tablename__#98AI-generated, human-checked. All changes pass mypy strict and the existing test suite. Tests added for
sa_typeinstances and__tablename__override/inheritance.Detailed examples for pyright diagnostics and before/after comparison: gist.