fix(pydantic): accept union type objects for stream_type hints#617
fix(pydantic): accept union type objects for stream_type hints#617majiayu000 wants to merge 1 commit intoapache:mainfrom
Conversation
| if typing.TYPE_CHECKING: | ||
| StreamingPydanticType = Any | ||
| else: | ||
| StreamingPydanticType = Union[Type["BaseModel"], Type[dict]] | ||
| if UnionType is not None: | ||
| StreamingPydanticType = Union[Type["BaseModel"], Type[dict], UnionType] |
There was a problem hiding this comment.
doing Any for typing checking will of course work and is cheating...
I think we shouldn't have a typing.TYPE_CHECKING part here, and instead I think the fix is line 74 + 75.
note I had to do this stream_type: Union[Type["BaseModel"], Type[dict], type, "types.UnionType"], for my pyright in the IDE to not complain -- i.e. the quotes part.
|
Thanks for the review! I've updated the code according to your suggestion:
The type definitions are now: # burr/integrations/pydantic.py
PartialType = Union[Type[pydantic.BaseModel], Type[dict], type, "types.UnionType"]
# burr/core/action.py
stream_type: Union[Type["BaseModel"], Type[dict], type, "types.UnionType"]This should satisfy pyright while properly accepting union type objects like |
|
This PR has been inactive for 20 days after receiving review feedback. Please mark it as ready for review when you have addressed the comments. |
skrawcz
left a comment
There was a problem hiding this comment.
Thanks for tackling this! The union type support for stream_type is a needed fix for issue #607, and the runtime behavior is correct.
The TYPE_CHECKING = Any approach disables type checking entirely — it accepts stream_type="hello" without complaint. I'd recommend replacing both TYPE_CHECKING blocks with a sys.version_info >= (3, 10) guard, which pyright understands natively:
if sys.version_info >= (3, 10):
StreamingPydanticType = Union[Type[BaseModel], Type[dict], types.UnionType]
else:
StreamingPydanticType = Union[Type[BaseModel], Type[dict]]This correctly narrows the type on 3.10+ while falling back gracefully on 3.9. I validated this with pyright against all the expected cases (union types, single types, dict, and invalid inputs).
| else: | ||
| StreamingPydanticType = Union[Type["BaseModel"], Type[dict]] | ||
| if UnionType is not None: | ||
| StreamingPydanticType = Union[Type["BaseModel"], Type[dict], UnionType] |
There was a problem hiding this comment.
The TYPE_CHECKING = Any pattern effectively disables the type checker for stream_type. Replace with:
if sys.version_info >= (3, 10):
StreamingPydanticType = Union[Type["BaseModel"], Type[dict], types.UnionType]
else:
StreamingPydanticType = Union[Type["BaseModel"], Type[dict]]sys is already imported in this file. Pyright natively narrows on sys.version_info checks.
| else: | ||
| PartialType = Union[Type[pydantic.BaseModel], Type[dict]] | ||
| if UnionType is not None: | ||
| PartialType = Union[Type[pydantic.BaseModel], Type[dict], UnionType] |
There was a problem hiding this comment.
Same issue — replace the TYPE_CHECKING = Any pattern with the sys.version_info guard for PartialType.
| @@ -22,6 +22,7 @@ | |||
| import types | |||
| import typing | |||
| from typing import ( | |||
There was a problem hiding this comment.
Nit: the Any import can be removed if switching to the sys.version_info approach.
Summary
Testing