Fix to allow Literal and Union via | in csp annotations #476
Conversation
Signed-off-by: Nijat Khanbabayev <[email protected]>
NeejWeej
requested review from
AdamGlustein,
alexddobkin,
robambalu and
svatasoiu
as code owners
Signed-off-by: Nijat Khanbabayev <[email protected]>
NeejWeej
force-pushed
the
nk/ast_fix
branch
2 times, most recently
from
235f6a0 to
902f3ce
Compare
…via pydantic Signed-off-by: Nijat Khanbabayev <[email protected]>
| @classmethod | ||
| def from_dict(cls, json: dict): | ||
| def from_dict(cls, json: dict, use_pydantic: bool = False): | ||
| if use_pydantic: |
There was a problem hiding this comment.
Rather than an arg can we base this on whether we are using pydantic type checking ( which defaults to true now )?
My only concern is speed. If we can do some timing tests and pydantic is as fast as the current code, I would just default it to use pydantic ( and eventually remove the old impl )
There was a problem hiding this comment.
The old way is a bit permissive and there will be some inconsistencies that might be annoying to fix, so I figured itd be easier to make it a flag for now. But eventually would like to switch to pydantic if its performant enough
| return cls.type_adapter().validate_python(json) | ||
| return cls._obj_from_python(json, cls) | ||
|
|
||
| def to_dict_depr(self): |
There was a problem hiding this comment.
should we have an equivalent pydantic to_dict?
There was a problem hiding this comment.
@robambalu Should we just remove to_dict_depr now?
| return cls._NORMALIZED_TYPE_MAPPING.get(CspTypingUtils.get_origin(typ), typ) | ||
| elif CspTypingUtils.is_union_type(typ): | ||
| return object | ||
| elif CspTypingUtils.is_literal_type(typ): |
There was a problem hiding this comment.
I realize we just moved code here, but looks like we can probably use normalized_type_to_actual_python_type here?
There was a problem hiding this comment.
But literal takes instances of types, not types
(so like Literal[7] versus Literal[int]) and normalized_type_to_actual_python_type doesnt seem to actually pull the true base class out except in this case for Literal
fixes #475