You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@hamilton.apache.org by "breken-ai (via GitHub)" <gi...@apache.org> on 2026/09/26 23:14:19 UTC

[PR] Fix FunctionInputOutputTypeChecker crash on Callable, type[...] and column[...] annotations [hamilton]

breken-ai opened a new pull request, #1739:
URL: https://github.com/apache/hamilton/pull/1739

   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.
   
   ```python
   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` (10a67af4, 2024-03-11).
   
   ## Checklist
   
   - [x] PR has an informative and human-readable title (this will be pulled into the release notes)
   - [x] Changes are limited to a single goal (no scope creep)
   - [x] Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
   - [x] 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](https://www.apache.org/legal/generative-tooling.html)):** 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](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@hamilton.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org