-
-
Notifications
You must be signed in to change notification settings - Fork 88
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
fix: handle unions properly #412
Conversation
Is this ready now? |
Unfortunately, no. |
I'm a bit stuck on this due to the way optional collection types are handled in pydantic v1. from typing import Optional
from pydantic import BaseModel
OptionalList = Optional[list[str]]
OptionalStr = Optional[str]
class Foo(BaseModel):
optional_list: OptionalList
optional_list_model_field = Foo.__fields__["optional_list"]
sub_field = optional_list_model_field.sub_fields[0]
print(sub_field.type) # gives 'str' but we want 'list[str]' Due to the way the handling of the unions is being changed in this PR, we want to get |
Hi. Is this still work in progress ? Would it be possible to fix this for Pydantic v2 as an intermediate measure ? |
Yeah, this is still in progress due to pydantic v1 tests failing. I think it might be possible to do this only for v2, though ideally, I'd like it to be done in such a way that we don't have to maintain a separate |
Pull Request Checklist
Description
This PR fixes the handling of unions. The use of
unwrap_args
would result in the incorrect creation of children for the field meta. It would just return a random instance of the union and it fails completely in the case of unions of collection types likelist[int] | list[str]
whereunwrap_args
might return(int,)
fortype_args
when in fact we want(list[int], list[str])
.This also fixes the parsing of the model type for
batch_factory_type
i.e. types likelist[SomeModel]
. Before, the model was taken from the value oftype_args
but that gives the incorrect value in the cases of unions likelist[SomeModel] | int
due to the change in the behavior oftype_args
. Instead we should just directly parse the model type from theunwrapped_type
since theis_batch_factory_type
is checked on theunwrapped_annotation
anyway.Close Issue(s)