-
Notifications
You must be signed in to change notification settings - Fork 246
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
Validate enum values against other possible forms of expected values #1502
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #1502 will not alter performanceComparing Summary
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1502 +/- ##
==========================================
- Coverage 90.21% 89.39% -0.83%
==========================================
Files 106 112 +6
Lines 16339 17966 +1627
Branches 36 40 +4
==========================================
+ Hits 14740 16060 +1320
- Misses 1592 1886 +294
- Partials 7 20 +13
... and 53 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
This issue lies mostly in validating JSON values. Take the case in the issue as an example: from enum import Enum
class A(Enum):
a = 1, 2
serialised = RootModel[A](A.a).model_dump_json()
parsed = RootModel[A].model_validate_json(serialised) # fail What happens here is that My initial commit does fix this issue by adding class Model(BaseModel):
x: int
@model_serializer
def ser_model(self):
return [self.x]
class A(Enum):
a = 2, 3
b = Model(x=1)
serialised = RootModel[A](A.b).model_dump_json() # [1]
parsed = RootModel[A].model_validate_json(serialised) # fail To fix this issue completely, we need to know the serialised form of all enum values and add them to the validator. My initial fix solves the problem because tuples are serialised as JSON arrays and casting tuples to lists covers this case. The problem for me now is that I don't know how to find the corresponding serialisers given a By the way, the following case is expected: class MyEnum(Enum):
a = 1, 2
b = [1, 2]
v = SchemaValidator(core_schema.enum_schema(MyEnum, list(MyEnum.__members__.values())))
assert v.validate_json('[1, 2]') is MyEnum.a Since both |
I think you might want to be looking in |
Hi @davidhewitt, I've finally come up with a seemingly generalisable solution, but I would like to check with you and see if I missed out anything. The idea is basically that we create a separate lookup in # root model with enum round trip
class Model(BaseModel):
x: int
@model_serializer
def ser_model(self):
return [self.x]
class A(Enum):
e = "1+2j"
a = 2, 3
b = complex(1, 2)
c = datetime.datetime.fromisoformat("2024-01-01T00:00:00Z")
d = Model(x=2)
f = float('inf')
for v in A.__members__.values():
RootModel[A].model_validate_json(RootModel[A](v).model_dump_json())
# base model with enum round trip
class M(BaseModel):
v: A
#model_config = ConfigDict(ser_json_inf_nan='strings', use_enum_values=True)
for v in A.__members__.values():
M.model_validate_json(M(v=v).model_dump_json()) I'm not quite sure about calling At the same time, I have some questions regarding implementations around pydantic_core._pydantic_core.ValidationError: 1 validation error for M
v
Input should be '1+2j', (2, 3), (1+2j), datetime.datetime(2024, 1, 1, 0, 0, tzinfo=datetime.timezone.utc), Model(x=2) or inf [type=enum, input_value='Infinity', input_type=str] because the serialized value in By the way, I'm still listing the original values in the error message instead of listing them in the JSON form because I'm not sure if showing the JSON form would be confusing. Sorry if this is not very clear because I'm in a bit of a rush, but please let me know what you think. Thank you! |
please review |
Change Summary
Validate enum values against other possible forms of expected values, specifically tuples. The problem is actually bigger than what the initial commit solves. Please see my comment.
Related issue number
Fixes pydantic/pydantic#10629
Checklist
pydantic-core
(except for expected changes)Selected Reviewer: @sydney-runkle