Enable Typing and generics based typing to support PEP585 - #727
Enable Typing and generics based typing to support PEP585#727arhamchopra wants to merge 3 commits into
Conversation
| return True | ||
| if isinstance(arg, SnapType): | ||
| return arg.ts_type.typ is inp_def_type | ||
| return arg.ts_type.typ == ContainerTypeNormalizer.normalize_type(inp_def_type) |
There was a problem hiding this comment.
why is the conversion needed at this point, wouldnt the input def type be normalized already
There was a problem hiding this comment.
Good question — but no, it isn't already normalized at this point. Only output defs get normalized (base_parser.py wraps them in ContainerTypeNormalizer.normalize_type); scalar input defs are stored with the raw evaluated annotation (InputDef(arg.arg, typ, ...)), and the AST-level TypeAnnotationNormalizerTransformer deliberately skips Subscript nodes (visit_Subscript returns them unchanged), so a PEP 585 list[str] reaches the resolver un-normalized. Verified at runtime: inp_def_type arrives here as list[str], not typing.List[str].
It's load-bearing too — without the normalize_type here, a union-typed snap like Optional[list[str]] fails to match in the non-pydantic resolver.
Your question also surfaced something we'd missed: the sibling SnapType check in _rec_validate_container_and_resolve_tvars had the same raw == comparison left un-normalized, so container snaps (snapped: list[str]) failed whenever pydantic was disabled (CSP_PYDANTIC=). Fixed in 504c79c, plus regression tests for list/dict/set and the union case (test_snap_builtin_generic_* in test_dynamic.py).
There was a problem hiding this comment.
im a bit concerned about performance implications of this, its hard to tell from just reading this code snippet how often this new expensive method is going to get invoked ( its certainly in a method thats called on every single node / graph invocation, so it will get called a ton )
There was a problem hiding this comment.
I understand, I changed it to change the type of args at function parsing time instead. Now whenever the functions are parsed, the types are normalized as well.
Signed-off-by: Arham Chopra <arham.chopra@cubistsystematic.com>
Signed-off-by: Arham Chopra <arham.chopra@cubistsystematic.com>
Signed-off-by: Arham Chopra <arham.chopra@cubistsystematic.com>
| else: | ||
| typ = self._eval_expr(arg.annotation) | ||
| arg_kind, basket_kind = self._resolve_input_type_kind(typ) | ||
| if arg_kind == ArgKind.SCALAR: |
There was a problem hiding this comment.
do we not need to convert edge types as well?
There was a problem hiding this comment.
edge types get handled, when we do the TsType.__class_getitem__ all the edge types are normalized. The normalize_type method calls canonicalize_builtin_generics
Motivation
PEP 585 — and the tooling that enforces it (ruff rule
UP006, pyupgrade, …) — rewrites the classictypinggenerics to their builtin equivalents in annotations:typing.List[X]→list[X],typing.Dict[K, V]→dict[K, V], and so on.csp's type-introspection layer treated a time-series built from a builtin generic as a different
type from the same time-series built from the
typingform:Because
csp.const([...])(and type inference generally) emits thetypingform while a modernizedannotation becomes the builtin form, any code that compares time-series types with a strict
==brokeafter a routine
List→listautoformat. Concretely, incsp-gateway,GatewayChannels.set_channelvalidates an edge against the declared channel type with
==:This produced 258 such failures across the
csp-gatewaytest suite. The clean fix belongs in csp:ts[list[X]]andts[typing.List[X]]should be the same type.Root cause
TsType.__class_getitem__normalizes its parameter throughContainerTypeNormalizer.normalize_type, but for an already-parametrized generic the normalizer returnedthe type unchanged, so
list[int]andtyping.List[int]yielded two distinctProtocol[...]specializations that were neither
==nor hash-equal.csp's graph-wiring / type-resolution path already treated the two origins as equivalent (via
CspTypingUtils._ORIGIN_COMPAT_MAPandget_origin, which maplist→typing.List, etc.), which iswhy graphs ran fine — this was an internal inconsistency in how
TsTypeobjects were built andcompared, not a real semantic incompatibility.
What this PR does
Canonicalizes builtin (PEP 585) container generics to their
typingequivalents, recursively, so bothspellings normalize to a single representation that compares and hashes equal.
Direction chosen: builtin →
typing, because that is the representation csp inference already emits, soit is the lowest-blast-radius option. (Moving the canonical form toward the builtins is the more "modern"
long-term direction and is left for a follow-up — see Limitations.)
Specifically,
ContainerTypeNormalizer:list/set/dict/tuple→typing.List/Set/Dict/Tupleat any nestingdepth.
X | Noneandtyping.Optional[...]) and through preserved wrappers(
typing.Mapping,FastList,Callable,csp.typingnumpy array types, custom generics): builtincontainers nested inside them are canonicalized while the wrapper keeps its own origin/flavor.
list["T"]) to a stabletyping.ForwardRef(matching how
typing.List["T"]is stored) rather than minting a freshTypeVaron each call.to preserve object identity for callers.
It also makes
csp.snap/csp.snapkeyscalar validation normalize the expected type and compare with==instead ofis(pydantic_types.make_snap_validatorandinstantiation_type_resolver._is_scalar_value_matching_spec). This keeps container-typed snaps workingregardless of the
listvstyping.Listspelling, and also fixes a pre-existing failure where a snapinto a builtin-form-annotated scalar (
x: list[int]) was already rejected.Behavior
Current state
csp/tests/impl/types/test_tstype.py: equality + hash parity for all fourcontainers, deep nesting, unions, Mapping/FastList/Callable recursion,
ForwardRefargs, identitypreservation, empty tuple, and a node-binding end-to-end test; plus a
csp.snapbuiltin-generic scalartest in
csp/tests/test_dynamic.py.Limitations / explicitly out of scope
ts[list] != ts[typing.List](likewisedict/set/tuple). Their natural canonical direction is the opposite — empty/untyped inference (csp.const([]))yields the builtin
list, nottyping.List— and normalizing them touches the C++ actual-type boundary(
normalized_type_to_actual_python_type(typing.List)returnstyping.List, not thelistclass). Thisis deferred to a follow-up to keep this change focused and low-risk.
collections.abc.Mapping[...]vstyping.Mapping[...], orcollections.abc.Callable[...]vstyping.Callable[...], remain distinct(they are distinct at the Python level, and csp only bridges the four standard containers via
_ORIGIN_COMPAT_MAP). Builtin containers nested inside such wrappers are still canonicalized; only theouter abc-vs-typing distinction remains.
ts[int | None] == ts[typing.Optional[int]]) already compared equal andare unchanged.
References
X | YUP006(non-pep585-annotation) — the modernization that surfaces thiscsp/impl/types/container_type_normalizer.py,csp/impl/types/pydantic_types.py,csp/impl/types/instantiation_type_resolver.py