Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support frozenset[...] generic #42

Closed
HamiltonRepoMigrationBot opened this issue Feb 26, 2023 · 2 comments
Closed

Support frozenset[...] generic #42

HamiltonRepoMigrationBot opened this issue Feb 26, 2023 · 2 comments
Labels
migrated-from-old-repo Migrated from old repository triage label for issues that need to be triaged.

Comments

@HamiltonRepoMigrationBot
Copy link
Collaborator

Issue by elijahbenizzy
Wednesday Aug 10, 2022 at 20:26 GMT
Originally opened as stitchfix/hamilton#175


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.

@HamiltonRepoMigrationBot HamiltonRepoMigrationBot added the triage label for issues that need to be triaged. label Feb 26, 2023
@HamiltonRepoMigrationBot
Copy link
Collaborator Author

Comment by elijahbenizzy
Wednesday Aug 10, 2022 at 20:28 GMT


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 elijahbenizzy added the migrated-from-old-repo Migrated from old repository label Feb 26, 2023
elijahbenizzy added a commit that referenced this issue Aug 15, 2023
@elijahbenizzy
Copy link
Collaborator

OK, this is not valid, as frozenset is not actually a subclass (nor a superclass) of Set. Fundamentally they have different behaviors, and you can't really pass one and expect another

elijahbenizzy added a commit that referenced this issue Aug 15, 2023
Inspired by #42. Note that we make the design decision to not error out
when we do a subclass check against generics. Rather, we return false.
elijahbenizzy added a commit that referenced this issue Aug 15, 2023
Inspired by #42. Note that we make the design decision to not error out
when we do a subclass check against generics. Rather, we return false.
We then change the error message to show how they can proceed if their
subclass check returns False.
elijahbenizzy added a commit that referenced this issue Aug 15, 2023
Inspired by #42. Note that we make the design decision to not error out
when we do a subclass check against generics. Rather, we return false.
We then change the error message to show how they can proceed if their
subclass check returns False.
elijahbenizzy added a commit that referenced this issue Aug 15, 2023
Inspired by #42. Note that we make the design decision to not error out
when we do a subclass check against generics. Rather, we return false.
We then change the error message to show how they can proceed if their
subclass check returns False.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migrated-from-old-repo Migrated from old repository triage label for issues that need to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants