Skip to content

Fix FunctionInputOutputTypeChecker crash on Callable, type[...] and column[...] annotations - #1739

Open
breken-ai wants to merge 1 commit into
apache:mainfrom
breken-ai:fix/check-instance-unchecked-generics
Open

breken-ai wants to merge 1 commit into
apache:mainfrom
breken-ai:fix/check-instance-unchecked-generics

Conversation

@breken-ai

Copy link
Copy Markdown
Contributor

Closes #1738.

With lifecycle.FunctionInputOutputTypeChecker enabled, a node annotated with Callable[[int], int], type[int], frozenset[str], Iterator[int] or Hamilton's own column[pd.Series, float] crashes the run, even when the value is correct.

def adder() -> Callable[[int], int]:
    return lambda x: x + 1

def spend() -> column[pd.Series, float]:
    return pd.Series([1.0, 2.0])

dr = driver.Builder().with_modules(mod).with_adapters(default.FunctionInputOutputTypeChecker()).build()
dr.execute(["adder"])  # TypeError: isinstance() argument 2 cannot be a parameterized generic
dr.execute(["spend"])  # TypeError: Subscripted generics cannot be used with class and instance checks

Why: htypes.check_instance first checks isinstance(obj, origin), then walks the type arguments only for dicts, lists, sets and tuples. Any other parameterized generic falls out of that block to the final return isinstance(obj, type_), with type_ still parameterized, and isinstance raises. This is the same crash as #1736, which was for PEP 604 unions, but it happens in a different branch.

Why the existing tests missed it: the check_instance tests only use list[...], dict[...], unions and Literal. Each of those returns before the final isinstance.

Changes

  • hamilton/htypes.py, check_instance: when a parameterized generic has already matched its origin and isn't one of the element-checked containers, return True instead of calling isinstance on the parameterized type. A value whose type doesn't match the origin is still rejected by the existing not isinstance(obj, origin) check.
  • tests/test_type_utils.py: test_check_instance_with_other_parameterized_generics covers matching values for typing.Callable[[int], int], type[int], frozenset[str], typing.Iterator[int], typing.Sequence[int] (a range) and htypes.column[pd.Series, float]. It also covers origin mismatches that must still return False.
  • tests/lifecycle/test_default.py:
    • a driver test with the adapter, where the nodes return and consume Callable[[int], int], frozenset[str] and column[pd.Series, float]
    • a test that a list returned for frozenset[str] is still rejected with the adapter's own TypeError

How I tested this

  • On unmodified main (f15267da, Python 3.12):
    • test_check_instance_with_other_parameterized_generics fails with TypeError: Subscripted generics cannot be used with class and instance checks.
    • test_function_input_output_type_checker_handles_other_parameterized_generics fails with the same error, raised from run_after_node_execution.
    • The rejection test passes on both main and the fix.
  • With the fix, all 3 pass.
  • pytest tests/test_type_utils.py tests/lifecycle tests/test_hamilton_driver.py tests/test_base.py tests/function_modifiers tests/test_function_modifiers.py tests/test_end_to_end.py: 791 passed, 1 skipped.
  • ruff check and ruff format --check (0.16.9, the pre-commit pin) are clean, and git diff --check is clean.

Notes

  • Types such as frozenset[str] and Iterator[int] get an origin-only check: their elements aren't validated. That is the most check_instance can do without consuming an iterator, and it matches how the adapter treats Callable. I kept the element-checked set (dict, list, set, tuple) unchanged so this PR stays one change.
  • The fall-through has been there since check_instance moved to htypes (10a67af, 2024-03-11).

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output): no new functions
  • Placeholder code is flagged / future TODOs are captured in comments: none added
  • Project documentation has been updated if adding/changing functionality: no documented behavior changes

AI disclosure (per the ASF Generative Tooling guidance): an AI coding tool (Claude Code, Claude Opus 5.5) wrote this change through the breken-ai account. The tool found the bug and wrote the fix, the tests and this description. The commit carries a Generated-by: trailer. The diff is a small original change to existing Hamilton code and includes no third-party material. The red/green runs above are real and can be re-run from the diff. If you would rather not take AI-assisted contributions here, say so and I will close this.

🤖 Generated with Claude Code

…nd column annotations

check_instance only walks the arguments of dicts, lists, sets and tuples.
For any other parameterized generic -- Callable[[int], int], type[int],
frozenset[str], Iterator[int], or Hamilton's own column[pd.Series, float] --
it matched the origin and then fell through to isinstance(obj, type_), which
raises TypeError for a parameterized generic. With the type-checker adapter
on, any node annotated this way crashed the run even for correct values.

Once the origin has matched, accept the value for these generics.

Generated-by: Claude Code (Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

FunctionInputOutputTypeChecker crashes on Callable[...], type[...], frozenset[...] and column[...] annotations

1 participant