Repository navigation
support optional annotated field with default value - #862
proever wants to merge 4 commits into
Conversation
Sorry, something went wrong.
|
looks like some (many) tests are failing, I'll try to fix them! Is this feature something that should work with Pydantic v1 too? I'm not too familiar with it. |
Sorry, something went wrong.
|
I've just hit exactly this problem; is there any chance you could update your MR please @proever? |
Sorry, something went wrong.
|
Would argue this PR fixes a bug and is not a FR pydantic validators is a core reason for the existence of both pydantic and SQLModel. SQLModel currently only supports validators using decorator-pattern Which is neither DRY nor reusable SQLModel does not support reusable Validators using annotated-pattern This PR would fix this. Example pydantic validator def validator_is_dead_or_cursed(value: int) -> int:
if value > 120:
msg_warn = (
f"{value!s} indicates this person very likely dead "
"or bibically cursed"
)
raise ValueError(msg_warn)
return value
NotDead = Annotated[int, pydantic.AfterValidator(validator_is_dead_or_cursed)]
Apply to Field age: Optional[NotDead] = Field(default=None, gt=0) or age: Annotated[Optional[int], pydantic.AfterValidator(validator_is_dead_or_cursed)] = Field(default=None, gt=0) Equivalent to age: Optional[int] = Field(default=None, gt=0, le=120) |
Sorry, something went wrong.
YuriiMotov
left a comment
There was a problem hiding this comment.
As I see it, this PR is basically about fixing the following scenario:
MyType: TypeAlias = Annotated[str, Field(max_length=5)]
class PriceModel(SQLModel, table=True):
id: int | None = Field(primary_key=True, default=None)
working: MyType # VARCHAR(5)
not_working: Optional[MyType] = None # VARCHAR(255)You are going to open a Pandora's box :)
I think this PR only handles one specific case, but in general we need to merge all nested Field annotations and handle conflicts. This seems to be a lot of work and quite error-prone.
Although, it would be nice to support this one day.
I suggest we skip it for now and get back to this feature later.
Seems to be related: #1281
As for initial code example, you can make it working by moving | None inside Annotated:
price: Annotated[Decimal | None, Field(decimal_places=2, max_digits=9)] = None@proever, thanks for your interest and efforts!
Sorry, something went wrong.
| type_ = get_sa_type_from_field(field) | ||
| metadata = get_field_metadata(field) | ||
| if isinstance(type_, _AnnotatedAlias): | ||
| class_to_compare = type_.__origin__ |
There was a problem hiding this comment.
Rename to use_type?
Sorry, something went wrong.
|
Thanks for the interest! And thanks a lot for the help @YuriiMotov 🙌 I haven't checked the code in this PR, but I was trying to run the original example and it seems to work, so this might have been solved at some point. from decimal import Decimal
from typing import Annotated
from sqlmodel import Field, SQLModel
class PriceModel(SQLModel, table=True):
id: int | None = Field(primary_key=True, default=None)
price: Annotated[Decimal, Field(decimal_places=2, max_digits=9)] | None = None
price = PriceModel()
print(price)Given that, and as the tests are failing, I'll pass on this one for now. If you have any other issue, please create a new discussion with the example to replicate it. 🤓 For now, I'll close this one. Thanks! 🚀 |
Sorry, something went wrong.
Currently, it is not possible to specify a SQLModel with an Optional complex type, such as a Decimal, as follows:
Doing so results in the following error (see #67, #312) in get_sqlalchemy_type:
TypeError: issubclass() arg 1 must be a classThis PR attempts to fix this issue by adding a check in get_sqlalchemy_type for whether the type of the Field is a typing_extensions.AnnotatedAlias. If it is, instead of using the class of the AnnotatedAlias for the following comparisons (which results in the above error), it uses AnnotatedAlias.__origin__. Similarly, it infers the metadata from AnnotatedAlias.__metadata__ instead of the AnnotatedAlias itself.
In my testing, this approach seems to work well. I also added a simple test here that checks whether restrictions on an optional annotated field are enforced on the database side. It could probably be improved or extended.