Improved TypedDict parameters for field with generic default values - #2256
Improved TypedDict parameters for field with generic default values#2256ropmyung wants to merge 23 commits into
TypedDict parameters for field with generic default values#2256Conversation
…ull: Literal[True]
…estored `TextField` not nullable because it's not supported on actual database
There was a problem hiding this comment.
Pull request overview
This PR improves static typing for Tortoise ORM field constructors by typing common **kwargs via PEP 692 Unpack[TypedDict], aiming to improve IDE autocomplete and type-checker validation, especially around generic default values.
Changes:
- Introduces
TypedDictdefinitions (FieldKwargs,RelationalFieldKwargs,ManyToManyFieldKwargs, etc.) to describe common field constructor kwargs. - Updates overload signatures in
tortoise/fields/data.pyandtortoise/fields/relational.pyto useUnpack[...]instead of**kwargs: Any. - Adds a changelog entry documenting the typed
**kwargsimprovement.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tortoise/fields/relational.py | Types relation field constructor **kwargs with Unpack[TypedDict] and minor formatting changes. |
| tortoise/fields/data.py | Types data field constructor overload **kwargs with Unpack[TypedDict] and adjusts TextField constructor handling. |
| tortoise/fields/base.py | Adds TypedDict kwargs schemas and changes Field defaults typing to be generic. |
| CHANGELOG.rst | Documents the new typed **kwargs behavior for field constructors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **kwargs: Unpack[FieldKwargs], | ||
| ) -> None: | ||
| if primary_key or kwargs.get("pk"): | ||
| db_index = kwargs.pop("db_index") |
| from tortoise.fields.base import ( | ||
| Field, | ||
| FieldKwargs, | ||
| JSONFieldKwargs, | ||
| _FieldKwargsCommon, | ||
| _FieldKwargsNoPk, | ||
| ) |
| source_field: str | None | ||
| generated: bool | ||
| default: VALUE | ||
| db_default: VALUE | ||
| description: str | None |
| null: bool = False, | ||
| default: Any = None, | ||
| db_default: Any = DB_DEFAULT_NOT_SET, | ||
| default: VALUE = None, | ||
| db_default: VALUE = DB_DEFAULT_NOT_SET, | ||
| unique: bool = False, |
- trying fix for TypedDicct iinheritance error
…eld to only support leyword arguments
| with pytest.warns(DeprecationWarning, match=self.message): | ||
| f = fields.TextField(primary_key=True) | ||
| assert f.pk is True | ||
| # Positional arg goes to primary_key, so only TextField as PK warning |
There was a problem hiding this comment.
Ah, I tried removing the positional arguments for TextField but I found there are some issues so I restored that but forgot the comment...
| from typing_extensions import Self | ||
| from collections.abc import Awaitable | ||
|
|
||
| from typing_extensions import Self, TypedDict |
There was a problem hiding this comment.
It seems that python3.10 also has TypedDict in the typing module.
There was a problem hiding this comment.
But that does not support multiple inheritance with Generic, so I used typing-extensions for 3.10
There was a problem hiding this comment.
Would be better to add comment for why using TypedDict from typing-extensions.
|
Please roll back |
|
@waketzheng Done! |
| through_table = Table(self.field.through, schema=self.field.through_schema) | ||
| backward_key, forward_key = self.field.backward_key, self.field.forward_key | ||
| backward_field, forward_field = through_table[backward_key], through_table[forward_key] | ||
| backward_field, forward_field = ( |
There was a problem hiding this comment.
Don't change this line.
| CASCADE, | ||
| SET_NULL, | ||
| Field, | ||
| ManyToManyFieldKwargs, |
There was a problem hiding this comment.
How about putting the imports of ManyToManyFieldKwargs and RelationalFieldKwargs under TYPE_CHECKING, so that we can keep this as a single line?
| try: | ||
| from pydantic import BaseModel as _PydanticBaseModel | ||
| from pydantic._internal._model_construction import ModelMetaclass as _PydanticModelMetaclass | ||
| from pydantic._internal._model_construction import ( |
There was a problem hiding this comment.
Please leave this line unchanged.
| try: | ||
| from pydantic import BaseModel as _PydanticBaseModel | ||
| from pydantic._internal._model_construction import ModelMetaclass as _PydanticModelMetaclass | ||
| from pydantic._internal._model_construction import ( |
There was a problem hiding this comment.
Please leave this line unchanged.
Added
Genericto default parameter to TypedDict params for better typing.Read #2218 for details