Skip to content
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

Handle union and literal typing correctly in annotations #478

Merged
merged 4 commits into from
Mar 7, 2025

Conversation

NeejWeej
Copy link
Collaborator

@NeejWeej NeejWeej commented Mar 7, 2025

This fixes a few issues all related to handling Union and Literals in type annotations. Namely:

fixes #475

Previously, the AST parsing for type annotations was incorrectly converting "None" in the example above to be a TypeVar, and then when we called exec on the new transformed AST, an error was being thrown. We make sure that in the AST parsing, we only convert a string to a TypeVar.

fixes #481

We also handle the case with Literal, where before if you had Literal["a", "b"], "a" and "b" were incorrectly assumed to be CspTypeVar, when they should be the actual literal value. For adjusting the annotations, we stop now if we encounter a Literal annotation.

fixes #482

Related to the Literal and Union typing, when from_dict on csp.Struct was called, we were skipping handling Literal and Union. We update the type normalization code to not treat Literal as a generic container. In from_dict, a Union has no validation performed on it, since under the hood the type normalizer returns object. For Literal, we perform (as we were before in adjusting the annotations themselves) a very basic type check. Namely, we just make sure that we pick a suitable parent class for all the values of Literal (as the type normalization does), and just check it is that class. The tests added demonstrate this, that we don't actually check for Literal when calling from_dict, but just make sure the types line up. So, if Literal[1, 2] exists, then a string passed for that field would error. The nuances of this behavior are addressed in this specific test case: https://github.com/Point72/csp/pull/478/files#diff-16e10f816ba9638999d038fb8bd076b4021e901b60e548b1bc392b6191026687R3907

NeejWeej added 4 commits March 5, 2025 19:58
Signed-off-by: Nijat Khanbabayev <nijat.khanbabayev@cubistsystematic.com>
Signed-off-by: Nijat Khanbabayev <nijat.khanbabayev@cubistsystematic.com>
…via pydantic

Signed-off-by: Nijat Khanbabayev <nijat.khanbabayev@cubistsystematic.com>
Signed-off-by: Nijat Khanbabayev <nijat.khanbabayev@cubistsystematic.com>
@timkpaine timkpaine changed the title Nk/handle union literal typing Handle union literal typing Mar 7, 2025
@timkpaine timkpaine added type: enhancement Issues and PRs related to improvements to existing features type: bug Concrete, reproducible bugs and removed type: enhancement Issues and PRs related to improvements to existing features labels Mar 7, 2025
@NeejWeej NeejWeej changed the title Handle union literal typing Handle union literal typing correctly in annotations Mar 7, 2025
@NeejWeej NeejWeej changed the title Handle union literal typing correctly in annotations Handle union and literal typing correctly in annotations Mar 7, 2025
@NeejWeej NeejWeej merged commit 81483c8 into main Mar 7, 2025
27 checks passed
@NeejWeej NeejWeej deleted the nk/handle_union_literal_typing branch March 7, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Concrete, reproducible bugs
Projects
None yet
4 participants