Skip to content

Enable Typing and generics based typing to support PEP585 - #727

Open
arhamchopra wants to merge 3 commits into
mainfrom
ac/ruff
Open

Enable Typing and generics based typing to support PEP585#727
arhamchopra wants to merge 3 commits into
mainfrom
ac/ruff

Conversation

@arhamchopra

@arhamchopra arhamchopra commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Motivation

PEP 585 — and the tooling that enforces it (ruff rule UP006, pyupgrade, …) — rewrites the classic
typing generics 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 typing form:

ts[list[int]] == ts[typing.List[int]]              # False  (should be True)
ts[dict[str, int]] == ts[typing.Dict[str, int]]    # False
hash(ts[list[int]]) == hash(ts[typing.List[int]])  # False

Because csp.const([...]) (and type inference generally) emits the typing form while a modernized
annotation becomes the builtin form, any code that compares time-series types with a strict == broke
after a routine Listlist autoformat. Concretely, in csp-gateway, GatewayChannels.set_channel
validates an edge against the declared channel type with ==:

TypeError: Edge type incorrect for my_list_channel:
  should be TsType[list[MyStruct]], found TsType[typing.List[MyStruct]]

This produced 258 such failures across the csp-gateway test suite. The clean fix belongs in csp:
ts[list[X]] and ts[typing.List[X]] should be the same type.

Root cause

TsType.__class_getitem__ normalizes its parameter through
ContainerTypeNormalizer.normalize_type, but for an already-parametrized generic the normalizer returned
the type unchanged, so list[int] and typing.List[int] yielded two distinct Protocol[...]
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_MAP and get_origin, which map listtyping.List, etc.), which is
why graphs ran fine — this was an internal inconsistency in how TsType objects were built and
compared, not a real semantic incompatibility.

What this PR does

Canonicalizes builtin (PEP 585) container generics to their typing equivalents, recursively, so both
spellings normalize to a single representation that compares and hashes equal.

Direction chosen: builtin → typing, because that is the representation csp inference already emits, so
it 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:

  • Remaps list / set / dict / tupletyping.List / Set / Dict / Tuple at any nesting
    depth
    .
  • Recurses through unions (X | None and typing.Optional[...]) and through preserved wrappers
    (typing.Mapping, FastList, Callable, csp.typing numpy array types, custom generics): builtin
    containers nested inside them are canonicalized while the wrapper keeps its own origin/flavor.
  • Canonicalizes a bare string argument inside a generic (list["T"]) to a stable typing.ForwardRef
    (matching how typing.List["T"] is stored) rather than minting a fresh TypeVar on each call.
  • Returns the original object unchanged when nothing actually changed, to avoid needless allocations and
    to preserve object identity for callers.

It also makes csp.snap / csp.snapkey scalar validation normalize the expected type and compare with
== instead of is (pydantic_types.make_snap_validator and
instantiation_type_resolver._is_scalar_value_matching_spec). This keeps container-typed snaps working
regardless of the list vs typing.List spelling, and also fixes a pre-existing failure where a snap
into a builtin-form-annotated scalar (x: list[int]) was already rejected.

Behavior

# before  ->  after
ts[list[int]] == ts[typing.List[int]]                    # False -> True
ts[dict[str, int]] == ts[typing.Dict[str, int]]          # False -> True
ts[set[int]] == ts[typing.Set[int]]                      # False -> True
ts[tuple[int, str]] == ts[typing.Tuple[int, str]]        # False -> True
ts[list[dict[str, int]]] == ts[List[Dict[str, int]]]     # False -> True  (nested)
ts[list[int] | None] == ts[Optional[List[int]]]          # False -> True  (union)
hash(ts[list[int]]) == hash(ts[typing.List[int]])        # False -> True
csp.const([1, 2, 3]).tstype == ts[list[int]]             # False -> True  (the csp-gateway symptom)

Current state

  • Full non-adapter test suite: 1079 passed / 25 skipped / 2 xfailed. Lint + format clean.
  • New regression coverage in csp/tests/impl/types/test_tstype.py: equality + hash parity for all four
    containers, deep nesting, unions, Mapping/FastList/Callable recursion, ForwardRef args, identity
    preservation, empty tuple, and a node-binding end-to-end test; plus a csp.snap builtin-generic scalar
    test in csp/tests/test_dynamic.py.

Limitations / explicitly out of scope

  • Bare, unparametrized aliases are not unified: ts[list] != ts[typing.List] (likewise dict/set/
    tuple). Their natural canonical direction is the opposite — empty/untyped inference (csp.const([]))
    yields the builtin list, not typing.List — and normalizing them touches the C++ actual-type boundary
    (normalized_type_to_actual_python_type(typing.List) returns typing.List, not the list class). This
    is deferred to a follow-up to keep this change focused and low-risk.
  • abc ↔ typing wrapper unification is not performed: e.g. collections.abc.Mapping[...] vs
    typing.Mapping[...], or collections.abc.Callable[...] vs typing.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 the
    outer abc-vs-typing distinction remains.
  • Top-level PEP 604 unions (ts[int | None] == ts[typing.Optional[int]]) already compared equal and
    are unchanged.

References

  • PEP 585 — Type Hinting Generics In Standard Collections
  • PEP 604 — Allow writing union types as X | Y
  • ruff rule UP006 (non-pep585-annotation) — the modernization that surfaces this
  • Touched: csp/impl/types/container_type_normalizer.py,
    csp/impl/types/pydantic_types.py, csp/impl/types/instantiation_type_resolver.py

Signed-off-by: Arham Chopra <arham.chopra@cubistsystematic.com>
@arhamchopra
arhamchopra marked this pull request as ready for review July 24, 2026 20:57
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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the conversion needed at this point, wouldnt the input def type be normalized already

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 )

@arhamchopra arhamchopra Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants