Skip to content

Fix type checks for explicit nodes, generics and unions - #71

Open
LucaMantani wants to merge 1 commit into
NNPDF:masterfrom
LucaMantani:check-types-explicit-node-generics
Open

LucaMantani wants to merge 1 commit into
NNPDF:masterfrom
LucaMantani:check-types-explicit-node-generics

Conversation

@LucaMantani

Copy link
Copy Markdown

resourcebuilder.check_types (and the parse_/allowed_keys checks in configparser) call isinstance(value, annotation) directly. That breaks with three common annotation patterns:

  1. @explicit_node resources. When a provider parameter is filled by an @explicit_node produce_X, the namespace still holds the ExplicitNode wrapper at check time (_process_requirement turns its .value into a node of its own). The real value doesn't exist yet, so any correct annotation fails:
    BadInputType: Value '<ExplicitNode ...>' is not of type Union, but of type 'ExplicitNode'.
    Provider outputs are never checked at this stage, so this change skips ExplicitNode values in the same way.
  2. Parametrised generics. def provider(settings: Mapping[str, Any]) crashes graph building with a bare TypeError: isinstance() argument 2 cannot be a parameterized generic, without naming the provider or parameter.
  3. X | None on Python 3.10–3.13. When the check fails, BadInputType itself raises AttributeError: 'types.UnionType' object has no attribute '__name__'.

Changes

  • New configparser.check_annotation(val, tp). It behaves like isinstance for classes and tuples, matches unions if any member matches, and checks generics against their origin class (type arguments are not validated). It's used by check_types, _parse_func and check_key_is_allowed.
  • check_types skips ExplicitNode values.
  • BadInputType names unions and generics via str(tp).
  • Tests in test_builder.py (explicit node + generic/union provider annotations, origin mismatch) and test_configparser.py (generic/union parse_ inputs). All three fail on master.

The full suite passes on Python 3.11 and 3.14, except test_collect and test_parallel_execute. Those two fail identically on master: recent distributed removed Client.register_worker_plugin.

Context: found in smefit, where adding type hints to a provider broke the graph.

🤖 Generated with Claude Code

check_types compared the ExplicitNode placeholder of an @explicit_node
resource against the annotation of the value it will produce, which can
never match. Skip it, as provider outputs are skipped.

Annotation checks (check_types, parse_ inputs, allowed_keys) now go
through check_annotation, which checks parametrised generics against
their origin class and unions member-wise, instead of raising a bare
TypeError from isinstance. BadInputType also names unions and generics
instead of failing with AttributeError on types.UnionType.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.

1 participant