-
Notifications
You must be signed in to change notification settings - Fork 333
Fix Pydantic transformer to support from __future__ import annotations
#3343
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 Pydantic transformer to support from __future__ import annotations
#3343
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3343 +/- ##
==========================================
- Coverage 81.40% 79.53% -1.87%
==========================================
Files 369 216 -153
Lines 29749 22697 -7052
Branches 2971 2974 +3
==========================================
- Hits 24217 18052 -6165
+ Misses 4738 3809 -929
- Partials 794 836 +42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| # Use Pydantic's field APIs to get properly resolved types | ||
| # This handles 'from __future__ import annotations' correctly | ||
| if hasattr(t, 'model_fields'): | ||
| # Pydantic v2 | ||
| fields = [(name, field_info.annotation) for name, field_info in t.model_fields.items()] | ||
| else: | ||
| # Pydantic v1 | ||
| fields = [(name, field_info.annotation) for name, field_info in t.__fields__.items()] |
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.
Can we just use fields = typing.get_type_hints(t).items() here?
|
Thank you for this PR! Could you please run |
af1cbdb to
108f519
Compare
Fixes #6694 When using `from __future__ import annotations`, the `__annotations__` dictionary contains string literals instead of actual type objects. This was causing the PydanticTransformer to fail to recognize standard Python types (str, int, float, etc.) in BaseModel fields, resulting in fallback to PickleFile serialization. Changes: - Updated `PydanticTransformer.get_literal_type()` to use Pydantic's `model_fields` API (v2) or `__fields__` (v1) instead of directly accessing `__annotations__` - These Pydantic APIs properly resolve string annotations to actual type objects, regardless of whether future annotations are enabled - Added comprehensive test suite covering simple types, complex types, nested models, and literal type structure validation The fix ensures compatibility with PEP 563 future annotations, which will become the default behavior in Python 3.14. Signed-off-by: Govert Verkes <[email protected]>
108f519 to
e02c0cf
Compare
machichima
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.
LGTM. Thank you!
|
Congrats on merging your first pull request! 🎉 |
Tracking issue
Closes flyteorg/flyte#6694
Why are the changes needed?
When using
from __future__ import annotations(PEP 563), the__annotations__dictionary contains string literals instead of actual type objects. The current implementation directly accesses__annotations__in thePydanticTransformer.get_literal_type()method, which causes standard Python types (str, int, float, etc.) in Pydantic BaseModel fields to be treated as unsupported types. This results in fallback to PickleFile serialization, causing warnings and inefficient serialization.This fix is important for compatibility with Python 3.14, where PEP 563 future annotations will become the default behavior.
What changes were proposed in this pull request?
Updated
PydanticTransformer.get_literal_type()inflytekit/extras/pydantic_transformer/transformer.pyto use Pydantic's built-in field APIs instead of directly accessing__annotations__:model_fieldsfor Pydantic v2__fields__for Pydantic v1 (backwards compatibility)These APIs properly resolve string annotations to actual type objects, regardless of whether future annotations are enabled.
Added comprehensive test suite in
test_future_annotations_bug.pywith 4 tests covering:How was this patch tested?
Setup process
from __future__ import annotationstransformer.pyScreenshots
Before fix:
After fix: No warnings, types correctly identified.
Check all the applicable boxes
Related PRs
N/A
Docs link
N/A (code fix only, no user-facing documentation changes needed)
Summary by Bito