-
-
Notifications
You must be signed in to change notification settings - Fork 113
Unpack type alisases when looking for NoDecode
#695
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
Conversation
bf696b4 to
bd44f2c
Compare
|
@Viicos let's continue the discussion about this here if you like. I had a quick look on the tests and I wasn't able to find any relevant test that we have nested annotations with NoDecode. Although I understand your concern regarding nesting, I am not sure I can replicate the issue. Do we actually care about the underling structure? I think we only need to identify that this field should not be decoded. I tried something like this and it seems to work. IntOrStr = TypeAliasType('IntOrStr', int | str)
A = TypeAliasType('A', Annotated[list[IntOrStr], NoDecode])this also works ListOrTuple = TypeAliasType('ListOrTuple', list[str] | tuple[str])
A = TypeAliasType('A', Annotated[ListOrTuple, NoDecode]) |
pydantic_settings/sources/utils.py
Outdated
| ) | ||
|
|
||
|
|
||
| def _get_field_metadata(field: Any) -> list[Any]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _get_field_metadata(field: Any) -> list[Any]: | |
| def _get_field_metadata(field: FieldInfo) -> list[Any]: |
What I was referring to by "nested" type aliases, is with something like: B = TypeAliasType('B', Annotated[list[str], NoDecode])
A = TypeAliasType('A', Annotated[B, ...])
class Settings(BaseSettings):
a: ATo be tested but I think right now this isn't taken into account? If not, let's go with what we have currently now, as it looks like this was a regression initially. |
|
@Viicos you are correct, we don't take into account this case. I worked a solution but I am not sure if this is the correct approach. Does it makes sense to gather all metadata together or we care only about NoDecode? def _get_annotation_metadata(annotation: Any) -> list[Any]:
if typing_objects.is_typealiastype(annotation) or typing_objects.is_typealiastype(get_origin(annotation)):
annotation = annotation.__value__
origin = get_origin(annotation)
if not typing_objects.is_annotated(origin):
return []
inner_annotation, *metadata = get_args(annotation)
if typing_objects.is_typealiastype(inner_annotation) or typing_objects.is_typealiastype(
get_origin(inner_annotation)
):
metadata += _get_annotation_metadata(inner_annotation)
return metadata
def _get_field_metadata(field: FieldInfo) -> list[Any]:
annotation = field.annotation
metadata = field.metadata
metadata += _get_annotation_metadata(annotation)
return metadata |
|
@tselepakis more generally (as I mentioned in #644 (comment)) the approach pydantic-settings is taking isn't really ideal and I fear we are going to face more limitations as time goes by. Let's merge as is and we can iterate later as ideally I'd like to make use of pydantic/typing-inspection#35 for this. Happy to merge once the remaining feedback is applied. |
bd44f2c to
30f7a4c
Compare
|
Sounds good to me. PR updated. I think we are good to go. |
Viicos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tselepakis!
Provide field metadata utility function to properly handle Annotated types.
The issue was mentioned here.
Fixes #715.