-
Notifications
You must be signed in to change notification settings - Fork 94
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
Support defining multiple optional fields with different types #416
Conversation
I want to add that this is a super-annoying bug and makes it difficult to use odmantic 1.0. Additionally this bug might have very nasty side-effects as it modifies the built-in Union[int, str] type. Though it might not be used a lot, but in case it is used by some other module, it will have the ability to break it in very creative ways imho. |
For people that cant wait for a released version, I use that to patch odmantic when building a docker image:
using a patch file --- a/odmantic/model.py
+++ b/odmantic/model.py
@@ -194,11 +194,9 @@
# generics is found
# https://github.com/pydantic/pydantic/issues/8354
if type_origin is Union:
- new_root = Union[
- int, str
- ] # We don't care about int,str since they will be replaced
- setattr(new_root, "__args__", new_arg_types)
- type_ = new_root # type: ignore
+ # as new_arg_types is a tuple, we can directly create a matching Union instance,
+ # instead of hacking our way around it: https://stackoverflow.com/a/72884529/3784643
+ type_ = Union[new_arg_types] # type: ignore
else:
type_ = GenericAlias(type_origin, new_arg_types) # type: ignore
return type_ |
…esulted in all optional fields being assigned the same type
2adee4d
to
2b3dcf6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #416 +/- ##
==========================================
- Coverage 99.18% 99.14% -0.04%
==========================================
Files 52 52
Lines 5045 5057 +12
Branches 712 712
==========================================
+ Hits 5004 5014 +10
- Misses 37 39 +2
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
If this patch works, this is amazing! I didn't find the workaround you use when I developed this feature. |
Thank you so much @netomi ! |
ty for merging the fix and making a new release right away. |
This fixes #407 .
The fix avoids the hack to dynamically generate a Union type instance which will just modify the existing Union[int, str] instance and re-use it for any type annotated with Union.