From bac8f90fa3e8d4b974b1f93f8201347ef769dd96 Mon Sep 17 00:00:00 2001 From: Ben Sowell Date: Tue, 28 Jan 2025 12:22:39 -0800 Subject: [PATCH] Change @context_params behavior to only pass explicit arguments. Previously it would attempt to pass missing arguments as part of kwargs, but this caused issues because Ray tried to interpret them. --- lib/sycamore/sycamore/context.py | 16 +++++----------- lib/sycamore/sycamore/tests/unit/test_context.py | 14 -------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/lib/sycamore/sycamore/context.py b/lib/sycamore/sycamore/context.py index 8f3bc597b..f53056794 100644 --- a/lib/sycamore/sycamore/context.py +++ b/lib/sycamore/sycamore/context.py @@ -117,19 +117,13 @@ def wrapper(*args, **kwargs): candidate_kwargs.pop(param, None) """ - If the function doesn't accept arbitrary kwargs, we don't want to use candidate_kwargs that aren't in - the function signature. + Remove candidate kwargs that aren't in the function signature. """ new_kwargs = {} - accepts_kwargs = any(param.kind == param.VAR_KEYWORD for param in sig.parameters.values()) - - if accepts_kwargs: - new_kwargs = candidate_kwargs - else: - for param in signature[len(args) :]: # arguments that haven't been passed as positional args - candidate_val = candidate_kwargs.get(param) - if candidate_val: - new_kwargs[param] = candidate_val + for param in signature[len(args) :]: # arguments that haven't been passed as positional args + candidate_val = candidate_kwargs.get(param) + if candidate_val: + new_kwargs[param] = candidate_val """ Put in user provided kwargs (either through decorator param or function call) diff --git a/lib/sycamore/sycamore/tests/unit/test_context.py b/lib/sycamore/sycamore/tests/unit/test_context.py index b34c50f79..498035137 100644 --- a/lib/sycamore/sycamore/tests/unit/test_context.py +++ b/lib/sycamore/sycamore/tests/unit/test_context.py @@ -110,17 +110,3 @@ def test_positional_args_and_context_args(): # Combine positional and kwarg assert "a b" == two_positional_args_method_with_kwargs("a", some_other_arg="b", context=context) - - -def test_positional_args_and_context_args_f_with_kwargs(): - context = Context( - params={"default": {"some_other_arg": "Aryn2", "some_unrelated_arg": "ArynZ"}}, exec_mode=ExecMode.LOCAL - ) - # Pickup 'some_other_arg' from context - assert "a Aryn2" == two_positional_args_method_with_kwargs(some_function_arg="a", context=context) - - # Should ignore context vars because of kwargs - assert "a b" == two_positional_args_method_with_kwargs(some_function_arg="a", some_other_arg="b", context=context) - - # Combine positional and kwarg - assert "a b" == two_positional_args_method_with_kwargs("a", some_other_arg="b", context=context)