hamilton icon indicating copy to clipboard operation
hamilton copied to clipboard

Inputs type annotation mismatch

Open zilto opened this issue 1 year ago • 0 comments

This is a scenario that happened to a user and we could surface better errors

Current behavior

Take a look at the type annotations for external_input. The current dataflow definition is valid because the types are deemed equivalent (Any is special, it is both a subclass and super class of all types, including MyClass)

from typing import Any

class MyClass:
   def __init__(self):
      ...

def foo(external_input: MyClass) -> int:
   return 1
   
def bar(external_input: Any) -> int:
   return 2
   
def join_nodes(foo: int, bar: int) -> int:
   return foo + bar

The problem happens when calling Driver.execute(...) and specifying external_input. If the user expects it to take Any and pass a value that's not MyClass, the input validation will say that there's a type mismatch without more details.

inputs = {"external_input": 31} 
dr.execute(["join_nodes"], inputs=inputs)

Additionally, because of the way visualizations are rendered, the two external_input will appear with the same type despite being annotated differently.

Also, the problem is specific to inputs. The following DAG wouldn't raise an error despite the object not matching the annotated type because there's no runtime type validation by default:

from typing import Any

class MyClass:
   def __init__(self):
      ...
      
def internal_input() -> MyClass:
   return 31

def foo(internal_input: MyClass) -> int:
   return 1
   
def bar(internal_input: Any) -> int:
   return 2
   
def join_nodes(foo: int, bar: int) -> int:
   return foo + bar
dr.execute(["join_nodes"])

Potential solutions

  • we could have better errors on input type validation
  • we could warn that nodes are annotated differently but with valid types at Builder.build() time
  • display the different type annotations in the visualization (I suggest against because it would be confusing since it can only be "one type at a time")

zilto avatar Oct 15 '24 14:10 zilto