hamilton icon indicating copy to clipboard operation
hamilton copied to clipboard

Support frozenset[...] generic

Open elijahbenizzy opened this issue 3 years ago • 1 comments

Short description explaining the high-level reason for the new issue.

Current behavior

This breaks:

def foo() -> frozenset[str]:
    ...

def bar(foo: Set[str]) -> ...:
    ...

But it should work. That said, the reverse isn't true -- we can't pass a set when expecting a frozenset :/

Stack Traces

I've gotten a few:

    raise e
../hamilton/hamilton/driver.py:57: in __init__
    self.graph = graph.FunctionGraph(*modules, config=config, adapter=adapter)
../hamilton/hamilton/graph.py:194: in __init__
    self.nodes = create_function_graph(*modules, config=self._config, adapter=adapter)
../hamilton/hamilton/graph.py:124: in create_function_graph
    add_dependency(n, node_name, nodes, param_name, param_type, adapter)
../hamilton/hamilton/graph.py:89: in add_dependency
    if not types_match(adapter, param_type, required_node.type):
../hamilton/hamilton/graph.py:50: in types_match
    elif custom_subclass_check(required_node_type, param_type):
../hamilton/hamilton/type_utils.py:45: in custom_subclass_check
    return issubclass(requested_type, param_type)```

## Screenshots
(If applicable)


## Steps to replicate behavior
1.

## Library & System Information
E.g. python version, hamilton library version, linux, etc.


# Expected behavior


# Additional context
Add any other context about the problem here.

elijahbenizzy avatar Aug 10 '22 20:08 elijahbenizzy

We should check all frozenset and Set[] implementations, as well as dict, list, etc... Also there's a python version issue as these only were allowed recently

elijahbenizzy avatar Aug 10 '22 20:08 elijahbenizzy

We are moving repositories! Please see the new version of this issue at https://github.com/DAGWorks-Inc/hamilton/issues/42. Also, please give us a star/update any of your internal links.

Note that everything else (slack community, pypi packages, etc...) will not change at all.

elijahbenizzy avatar Feb 26 '23 17:02 elijahbenizzy