From 6144bd4d150aa354aabde0b7a60fda69373d8539 Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Wed, 25 Mar 2026 19:43:10 +0000 Subject: [PATCH 1/7] call decorator.validate() in resolve.resolve so that delayed evaluation of extract_fields works; add unit test --- hamilton/function_modifiers/delayed.py | 5 ++++- tests/function_modifiers/test_delayed.py | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/hamilton/function_modifiers/delayed.py b/hamilton/function_modifiers/delayed.py index d22ef9231..0b30b67ed 100644 --- a/hamilton/function_modifiers/delayed.py +++ b/hamilton/function_modifiers/delayed.py @@ -169,7 +169,10 @@ def resolve(self, config: dict[str, Any], fn: Callable) -> NodeTransformLifecycl for key in self._optional_config: if key in config: kwargs[key] = config[key] - return self.decorate_with(**kwargs) + decorator = self.decorate_with(**kwargs) + if hasattr(decorator, "validate"): + decorator.validate(fn) + return decorator class resolve_from_config(resolve): diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index 4dcf11818..404376306 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -24,6 +24,7 @@ ResolveAt, base, extract_columns, + extract_fields, resolve, resolve_from_config, ) @@ -86,7 +87,8 @@ def test_dynamic_resolve_with_configs(): decorate_with=lambda cols_to_extract: extract_columns(*cols_to_extract), ) decorator_resolved = decorator.resolve( - {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, fn=test_dynamic_resolves + {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, + fn=test_dynamic_resolve_with_configs, ) # This uses an internal component of extract_columns # We may want to add a little more comprehensive testing @@ -157,3 +159,21 @@ def test_delayed_without_power_mode_fails(): {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_DISABLED}, fn=test_delayed_with_optional, ) + + +def test_dynamic_resolve_with_extract_fields(): + """Test that @resolve with @extract_fields calls validate() correctly.""" + + def my_function() -> dict[str, int]: + return {"a": 1, "b": 2} + + decorator = resolve( + when=ResolveAt.CONFIG_AVAILABLE, + decorate_with=lambda fields: extract_fields(fields), + ) + decorator_resolved = decorator.resolve( + {"fields": {"a": int, "b": int}, **CONFIG_WITH_POWER_MODE_ENABLED}, + fn=my_function, + ) + assert hasattr(decorator_resolved, "resolved_fields") + assert decorator_resolved.resolved_fields == {"a": int, "b": int} From 21174e58cee4ddf7ba26909401c23e68db416c91 Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Wed, 25 Mar 2026 19:59:20 +0000 Subject: [PATCH 2/7] fix new failures in delayed resolve tests by defining dummy functions with valid annotations within the test body --- tests/function_modifiers/test_delayed.py | 40 +++++++++++++++--------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index 404376306..09432aa29 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -17,6 +17,7 @@ from collections.abc import Callable +import pandas as pd import pytest from hamilton import settings @@ -69,12 +70,17 @@ def test_extract_and_validate_params_unhappy(fn: Callable): def test_dynamic_resolves(): + # Note: we use an empty DataFrame for validation only. This test would fail at runtime + # if we actually tried to execute the DAG because there are no columns "a" or "b" to extract. + def fn() -> pd.DataFrame: + return pd.DataFrame() + decorator = resolve( when=ResolveAt.CONFIG_AVAILABLE, decorate_with=lambda cols_to_extract: extract_columns(*cols_to_extract), ) decorator_resolved = decorator.resolve( - {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, fn=test_dynamic_resolves + {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, fn=fn ) # This uses an internal component of extract_columns # We may want to add a little more comprehensive testing @@ -83,12 +89,15 @@ def test_dynamic_resolves(): def test_dynamic_resolve_with_configs(): + def fn() -> pd.DataFrame: + return pd.DataFrame() + decorator = resolve_from_config( decorate_with=lambda cols_to_extract: extract_columns(*cols_to_extract), ) decorator_resolved = decorator.resolve( {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, - fn=test_dynamic_resolve_with_configs, + fn=fn, ) # This uses an internal component of extract_columns # We may want to add a little more comprehensive testing @@ -97,18 +106,15 @@ def test_dynamic_resolve_with_configs(): def test_dynamic_fails_without_power_mode_fails(): + def fn() -> pd.DataFrame: + return pd.DataFrame() + decorator = resolve( when=ResolveAt.CONFIG_AVAILABLE, decorate_with=lambda cols_to_extract: extract_columns(*cols_to_extract), ) with pytest.raises(base.InvalidDecoratorException): - decorator_resolved = decorator.resolve( - CONFIG_WITH_POWER_MODE_DISABLED, fn=test_dynamic_fails_without_power_mode_fails - ) - # This uses an internal component of extract_columns - # We may want to add a little more comprehensive testing - # But for now this will work - assert decorator_resolved.columns == ("a", "b") + decorator.resolve(CONFIG_WITH_POWER_MODE_DISABLED, fn=fn) def test_config_derivation(): @@ -125,6 +131,9 @@ def test_config_derivation(): def test_delayed_with_optional(): + def fn() -> pd.DataFrame: + return pd.DataFrame() + decorator = resolve( when=ResolveAt.CONFIG_AVAILABLE, decorate_with=lambda cols_to_extract, some_cols_you_might_want_to_extract=["c"]: ( @@ -133,7 +142,7 @@ def test_delayed_with_optional(): ) resolved = decorator.resolve( {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_ENABLED}, - fn=test_delayed_with_optional, + fn=fn, ) assert list(resolved.columns) == ["a", "b", "c"] resolved = decorator.resolve( @@ -142,12 +151,15 @@ def test_delayed_with_optional(): "some_cols_you_might_want_to_extract": ["d"], **CONFIG_WITH_POWER_MODE_ENABLED, }, - fn=test_delayed_with_optional, + fn=fn, ) assert list(resolved.columns) == ["a", "b", "d"] def test_delayed_without_power_mode_fails(): + def fn() -> pd.DataFrame: + return pd.DataFrame() + decorator = resolve( when=ResolveAt.CONFIG_AVAILABLE, decorate_with=lambda cols_to_extract, some_cols_you_might_want_to_extract=["c"]: ( @@ -157,14 +169,14 @@ def test_delayed_without_power_mode_fails(): with pytest.raises(base.InvalidDecoratorException): decorator.resolve( {"cols_to_extract": ["a", "b"], **CONFIG_WITH_POWER_MODE_DISABLED}, - fn=test_delayed_with_optional, + fn=fn, ) def test_dynamic_resolve_with_extract_fields(): """Test that @resolve with @extract_fields calls validate() correctly.""" - def my_function() -> dict[str, int]: + def fn() -> dict[str, int]: return {"a": 1, "b": 2} decorator = resolve( @@ -173,7 +185,7 @@ def my_function() -> dict[str, int]: ) decorator_resolved = decorator.resolve( {"fields": {"a": int, "b": int}, **CONFIG_WITH_POWER_MODE_ENABLED}, - fn=my_function, + fn=fn, ) assert hasattr(decorator_resolved, "resolved_fields") assert decorator_resolved.resolved_fields == {"a": int, "b": int} From ed185d8a8ba88f5d20fe83ca833dccb2d1f487c7 Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:19:17 +0000 Subject: [PATCH 3/7] correct probably typo in test name --- tests/function_modifiers/test_delayed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index 09432aa29..e8f674a44 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -105,7 +105,7 @@ def fn() -> pd.DataFrame: assert decorator_resolved.columns == ("a", "b") -def test_dynamic_fails_without_power_mode_fails(): +def test_dynamic_resolve_without_power_mode_fails(): def fn() -> pd.DataFrame: return pd.DataFrame() From 29f3edd577385046f46b41e42244c74c6cac526f Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Fri, 3 Apr 2026 12:47:22 +0100 Subject: [PATCH 4/7] add note above the 'defensive' hasattr check to explain its presence --- hamilton/function_modifiers/delayed.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hamilton/function_modifiers/delayed.py b/hamilton/function_modifiers/delayed.py index 0b30b67ed..2eb8725fe 100644 --- a/hamilton/function_modifiers/delayed.py +++ b/hamilton/function_modifiers/delayed.py @@ -170,8 +170,14 @@ def resolve(self, config: dict[str, Any], fn: Callable) -> NodeTransformLifecycl if key in config: kwargs[key] = config[key] decorator = self.decorate_with(**kwargs) + + # NOTE: cases where `decorator` has no `validate` method should be caught by type checkers + # since `decorate_with` is typed as `Callable[..., NodeTransformLifecycle]`. The following + # check allows non-conforming functions to be used with `resolve` without immediately + # throwing an error, which may be undesirable. if hasattr(decorator, "validate"): decorator.validate(fn) + return decorator From 42057561173f358773095432b18e37d462661f64 Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Fri, 3 Apr 2026 13:27:37 +0100 Subject: [PATCH 5/7] add additional tests for resolve + extract_fields; test resolve + arbitrary decorator --- tests/function_modifiers/test_delayed.py | 61 ++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index e8f674a44..ea2d4b1da 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -189,3 +189,64 @@ def fn() -> dict[str, int]: ) assert hasattr(decorator_resolved, "resolved_fields") assert decorator_resolved.resolved_fields == {"a": int, "b": int} + + +def test_resolve_from_config_with_extract_fields(): + """Test @resolve_from_config with @extract_fields calls validate() correctly.""" + + def fn() -> dict[str, int]: + return {"a": 1, "b": 2} + + decorator = resolve_from_config( + decorate_with=lambda fields: extract_fields(fields), + ) + decorator_resolved = decorator.resolve( + {"fields": {"a": int, "b": int}, **CONFIG_WITH_POWER_MODE_ENABLED}, + fn=fn, + ) + assert hasattr(decorator_resolved, "resolved_fields") + assert decorator_resolved.resolved_fields == {"a": int, "b": int} + + +def test_resolve_propagates_validate_failure(): + """Test that validate() failures are propagated through resolve.""" + + def fn() -> str: + return "not what you were expecting..." + + decorator = resolve( + when=ResolveAt.CONFIG_AVAILABLE, + decorate_with=lambda fields: extract_fields(fields), + ) + with pytest.raises(base.InvalidDecoratorException): + decorator.resolve( + {"fields": {"a": int, "b": int}, **CONFIG_WITH_POWER_MODE_ENABLED}, + fn=fn, + ) + + +def test_resolve_with_arbitrary_decorator(): + """Test behavior when decorate_with returns something that is not a NodeTransformLifecycle.""" + + # NOTE: do we actually want this test to pass? + + # A decorator that doesn't inherit from NodeTransformLifecycle (but still uses kwargs only) + class ArbitraryDecorator: + def __init__(self, a: int, b: int) -> None: + pass + + def __call__(self, f: Callable) -> Callable: + return f + + def fn() -> pd.DataFrame: + return pd.DataFrame() + + decorator = resolve( + when=ResolveAt.CONFIG_AVAILABLE, + decorate_with=lambda kwargs: ArbitraryDecorator(**kwargs), + ) + decorator_resolved = decorator.resolve( + {"kwargs": {"a": 1, "b": 2}, **CONFIG_WITH_POWER_MODE_ENABLED}, + fn=fn, + ) + assert isinstance(decorator_resolved, ArbitraryDecorator) From eca7fbf9a01ae0e4916a2939a872e951a6461072 Mon Sep 17 00:00:00 2001 From: jmarshrossney <17361029+jmarshrossney@users.noreply.github.com> Date: Fri, 3 Apr 2026 13:40:11 +0100 Subject: [PATCH 6/7] add one more test that uses parameterize_sources instead of extract_fields --- tests/function_modifiers/test_delayed.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index ea2d4b1da..84d553d29 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -26,6 +26,7 @@ base, extract_columns, extract_fields, + parameterize_sources, resolve, resolve_from_config, ) @@ -191,6 +192,26 @@ def fn() -> dict[str, int]: assert decorator_resolved.resolved_fields == {"a": int, "b": int} +def test_resolve_with_parameterize_sources(): + """Test that @resolve with @parameterize_sources calls validate() correctly.""" + + def fn(x: int, y: int) -> int: + return x + y + + decorator = resolve( + when=ResolveAt.CONFIG_AVAILABLE, + decorate_with=lambda: parameterize_sources(result_1={"x": "source_x", "y": "source_y"}), + ) + decorator_resolved = decorator.resolve( + {**CONFIG_WITH_POWER_MODE_ENABLED}, + fn=fn, + ) + assert "result_1" in decorator_resolved.parameterization + mapping = decorator_resolved.parameterization["result_1"] + assert mapping["x"].source == "source_x" + assert mapping["y"].source == "source_y" + + def test_resolve_from_config_with_extract_fields(): """Test @resolve_from_config with @extract_fields calls validate() correctly.""" From 020665b9e381d6f7ba3739b0dd0e5d9ced4cbba1 Mon Sep 17 00:00:00 2001 From: Stefan Krawczyk Date: Sat, 4 Apr 2026 14:51:25 +1100 Subject: [PATCH 7/7] Apply suggestion from @skrawcz --- tests/function_modifiers/test_delayed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/function_modifiers/test_delayed.py b/tests/function_modifiers/test_delayed.py index 84d553d29..b48fc92b9 100644 --- a/tests/function_modifiers/test_delayed.py +++ b/tests/function_modifiers/test_delayed.py @@ -249,7 +249,7 @@ def fn() -> str: def test_resolve_with_arbitrary_decorator(): """Test behavior when decorate_with returns something that is not a NodeTransformLifecycle.""" - # NOTE: do we actually want this test to pass? + # NOTE: we want to ensure we don't interfere with other decorators on functions. # A decorator that doesn't inherit from NodeTransformLifecycle (but still uses kwargs only) class ArbitraryDecorator: