Skip to content

Commit

Permalink
Fixes default parameter calculation
Browse files Browse the repository at this point in the history
The defaults overwrote the equals operator, which then failed on
cmoparison to .empty. This fixes it to use is Parameter.empty which is
a better approach in python.
  • Loading branch information
elijahbenizzy committed Feb 22, 2024
1 parent 2cfe00c commit a60ac14
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 4 deletions.
4 changes: 2 additions & 2 deletions hamilton/function_modifiers/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def test_function_signatures_compatible(
dummy_param_values = {
key: SENTINEL_ARG_VALUE
for key, param_spec in fn_signature.parameters.items()
if param_spec.default != inspect.Parameter.empty
if param_spec.default is not inspect.Parameter.empty
}
# Then we update with the dummy values. Again, replacing doesn't matter (we'll be mimicking it later)
dummy_param_values.update({key: SENTINEL_ARG_VALUE for key in fn_signature.parameters})
Expand Down Expand Up @@ -214,7 +214,7 @@ def wrapper_function(**kwargs):
final_kwarg_values = {
key: param_spec.default
for key, param_spec in inspect.signature(fn).parameters.items()
if param_spec.default != inspect.Parameter.empty
if param_spec.default is not inspect.Parameter.empty
}
final_kwarg_values.update(kwargs)
final_kwarg_values = does.map_kwargs(final_kwarg_values, self.argument_mapping)
Expand Down
2 changes: 1 addition & 1 deletion hamilton/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def _check_keyword_args_only(func: Callable) -> bool:
"""Checks if a function only takes keyword arguments."""
sig = inspect.signature(func)
for param in sig.parameters.values():
if param.default == inspect.Parameter.empty and param.kind not in [
if param.default is inspect.Parameter.empty and param.kind not in [
inspect.Parameter.KEYWORD_ONLY,
inspect.Parameter.VAR_KEYWORD,
]:
Expand Down
2 changes: 1 addition & 1 deletion hamilton/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class DependencyType(Enum):

@staticmethod
def from_parameter(param: inspect.Parameter):
if param.default == inspect.Parameter.empty:
if param.default is inspect.Parameter.empty:
return DependencyType.REQUIRED
return DependencyType.OPTIONAL

Expand Down
13 changes: 13 additions & 0 deletions tests/test_node.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import inspect
import sys
from typing import Any, Literal, TypeVar

Expand Down Expand Up @@ -104,3 +105,15 @@ def annotated_func(first: ArrayN[np.float64], other: float = 2.0) -> ArrayN[np.f
)
def test_tags_match_query(tags: dict, query: dict, expected: bool):
assert matches_query(tags, query) == expected


def test_from_parameter_default_override_equals():
class BrokenEquals:
def __eq__(self, other):
raise ValueError("I'm broken")

def foo(b: BrokenEquals = BrokenEquals()):
pass

param = DependencyType.from_parameter(inspect.signature(foo).parameters["b"])
assert param == DependencyType.OPTIONAL

0 comments on commit a60ac14

Please sign in to comment.