Fix json validation for NameEmail #8650
Conversation
CodSpeed Performance ReportMerging #8650 will not alter performanceComparing Summary
|
pydantic/networks.py
Outdated
| @@ -464,22 +464,15 @@ def __init__(self, name: str, email: str): | |||
| def __eq__(self, other: Any) -> bool: | |||
| return isinstance(other, NameEmail) and (self.name, self.email) == (other.name, other.email) | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
I am not sure if my approach is correct here.
Actually I don't know what this method is (can't find docs on the website). But seems deleting this method breaks the json schema generation on format. Currently unit test are failing and I need some guidance on fixing that.
I added support on {"email": "", "name": ""} schema for the input. Not sure if we should do that or just stick with plain str which is a bit easier to implement I guess.
There was a problem hiding this comment.
I think we can actually leave this here, that should help to resolve some of the issues with the failing tests.
tests/test_networks.py
Outdated
| 'foo bar', 'foobaR@example.com' | ||
| ) | ||
|
|
||
| assert Model.model_validate_json('{"v":{"name":"foo bar","email":"whatever <foobaR@example.com>"}}').v == NameEmail( |
There was a problem hiding this comment.
IDK if this case should be allowed or should raise an exception. Coz the implementation just override the name from dict and throw away parsed name.
There was a problem hiding this comment.
I don't think we want to support this case, as mentioned in my above comment.
Holi0317
force-pushed
the
8649-name-email-json
branch
from
f7a449c
to
2c239f1
Compare
pydantic/networks.py
Outdated
| @@ -464,22 +464,15 @@ def __init__(self, name: str, email: str): | |||
| def __eq__(self, other: Any) -> bool: | |||
| return isinstance(other, NameEmail) and (self.name, self.email) == (other.name, other.email) | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
I think we can actually leave this here, that should help to resolve some of the issues with the failing tests.
pydantic/networks.py
Outdated
| field_schema = handler(core_schema) | ||
| field_schema.update(type='string', format='name-email') | ||
| return field_schema | ||
|
|
||
| @classmethod | ||
| def __get_pydantic_core_schema__( |
There was a problem hiding this comment.
I think a more simple approach would be something like this:
@classmethod
def __get_pydantic_core_schema__(
cls,
_source: type[Any],
_handler: GetCoreSchemaHandler,
) -> core_schema.CoreSchema:
import_email_validator()
return core_schema.no_info_after_validator_function(
cls._validate,
core_schema.json_or_python_schema(
json_schema=core_schema.str_schema(),
python_schema=core_schema.union_schema(
[core_schema.is_instance_schema(cls), core_schema.str_schema()],
custom_error_type='name_email_type',
custom_error_message='Input is not a valid NameEmail',
),
),
)
tests/test_networks.py
Outdated
| 'foo bar', 'foobaR@example.com' | ||
| ) | ||
|
|
||
| assert Model.model_validate_json('{"v":{"name":"foo bar","email":"whatever <foobaR@example.com>"}}').v == NameEmail( |
There was a problem hiding this comment.
I don't think we want to support this case, as mentioned in my above comment.
Holi0317
force-pushed
the
8649-name-email-json
branch
from
2c239f1
to
ec18bb1
Compare
|
@sydney-runkle Thanks for your review. I've made the changes you request and simplify the test case a bit. |
Fixes #8649
Change Summary
Fixes json validation for
NameEmailtypeRelated issue number
#8649
Checklist