From 68c2be17cfdff4ce318242aeefcff2d5bb1b39d0 Mon Sep 17 00:00:00 2001 From: chrisfandrade16 Date: Thu, 12 Sep 2024 14:27:41 -0700 Subject: [PATCH 1/3] Remove usage of configure.defaults and modify configure.arguments to consider all cases --- workflow/lifecycle/configure.py | 32 +++++++++++++++++++++++--------- workflow/lifecycle/execute.py | 5 +---- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/workflow/lifecycle/configure.py b/workflow/lifecycle/configure.py index 068bd90..9858891 100644 --- a/workflow/lifecycle/configure.py +++ b/workflow/lifecycle/configure.py @@ -91,7 +91,7 @@ def loki(logger: Logger, config: Dict[str, Any]) -> bool: return status -def arguments(arguments: Optional[Dict[str, Any]]) -> List[str]: +def arguments(func: Callable[..., Any], work: Work) -> List[str]: """Return a list of CLI arguments. Args: @@ -101,14 +101,28 @@ def arguments(arguments: Optional[Dict[str, Any]]) -> List[str]: List[str]: List of CLI arguments """ args: List[str] = [] - if arguments: - for key, value in arguments.items(): - if not value: - args.append(f"{key}") - elif value and len(key) == 2: - args.append(f"{key} {value}") - elif value and len(key) > 2: - args.append(f"{key}={value}") + for parameter in func.params: + parameter_name_in_cli = parameter.opts[-1] + paraneter_name_in_function = parameter.name + parameter_value_in_work = work.parameters.get(paraneter_name_in_function, None) + + if parameter_value_in_work: + if isinstance(parameter, click.Argument): + # If argument, then the parameter is purely positional without a key + args.append(f"{parameter_value_in_work}") + elif isinstance(parameter, click.Option): + if not parameter_value_in_work: + # If is_flag=True, then the parameter is a flag and doesn't have a value + args.append(f"{parameter_name_in_cli}") + elif parameter_value_in_work and len(parameter_name_in_cli) == 2: + # Short options (often denoted by a single hyphen and a single letter like -r) + # are commonly written with a space between the option and its value + args.append(f"{parameter_name_in_cli} {parameter_value_in_work}") + elif parameter_value_in_work and len(parameter_name_in_cli) > 2: + # Long options (often denoted by two hyphens and a word like --recursive) + # are commonly written with an equals sign between the option and its value + args.append(f"{parameter_name_in_cli}={parameter_value_in_work}") + return args diff --git a/workflow/lifecycle/execute.py b/workflow/lifecycle/execute.py index 0595f22..553faff 100644 --- a/workflow/lifecycle/execute.py +++ b/workflow/lifecycle/execute.py @@ -36,13 +36,10 @@ def function(work: Work) -> Work: assert isinstance(work.function, str), "missing function to execute" func: Callable[..., Any] = validate.function(work.function) arguments: List[str] = [] - # Configure default values for the function - work = configure.defaults(func, work) - # Paramters to execute the function with parameters: Dict[str, Any] = work.parameters or {} if isinstance(func, click.Command): - arguments = configure.arguments(parameters) + arguments = configure.arguments(func, work) logger.info( f"executing: {func.name}.main(args={arguments}, standalone_mode=False)" ) From 4ced9fab98f30d1c02ec36c0f66241869a9e215b Mon Sep 17 00:00:00 2001 From: chrisfandrade16 Date: Thu, 12 Sep 2024 15:26:48 -0700 Subject: [PATCH 2/3] Check for is_flag with @click.options --- workflow/lifecycle/configure.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/workflow/lifecycle/configure.py b/workflow/lifecycle/configure.py index 9858891..2856b12 100644 --- a/workflow/lifecycle/configure.py +++ b/workflow/lifecycle/configure.py @@ -111,17 +111,22 @@ def arguments(func: Callable[..., Any], work: Work) -> List[str]: # If argument, then the parameter is purely positional without a key args.append(f"{parameter_value_in_work}") elif isinstance(parameter, click.Option): - if not parameter_value_in_work: + if hasattr(parameter, "is_flag") and parameter.is_flag: # If is_flag=True, then the parameter is a flag and doesn't have a value args.append(f"{parameter_name_in_cli}") - elif parameter_value_in_work and len(parameter_name_in_cli) == 2: - # Short options (often denoted by a single hyphen and a single letter like -r) - # are commonly written with a space between the option and its value - args.append(f"{parameter_name_in_cli} {parameter_value_in_work}") - elif parameter_value_in_work and len(parameter_name_in_cli) > 2: - # Long options (often denoted by two hyphens and a word like --recursive) - # are commonly written with an equals sign between the option and its value - args.append(f"{parameter_name_in_cli}={parameter_value_in_work}") + else: + if len(parameter_name_in_cli) == 2: + # Short options (often denoted by a single hyphen and a single letter like -r) + # are commonly written with a space between the option and its value + args.append( + f"{parameter_name_in_cli} {parameter_value_in_work}" + ) + elif len(parameter_name_in_cli) > 2: + # Long options (often denoted by two hyphens and a word like --recursive) + # are commonly written with an equals sign between the option and its value + args.append( + f"{parameter_name_in_cli}={parameter_value_in_work}" + ) return args From 42083664a817e4d4f59dd2c71a9137473b18e80b Mon Sep 17 00:00:00 2001 From: chrisfandrade16 Date: Thu, 12 Sep 2024 15:35:46 -0700 Subject: [PATCH 3/3] Add check if Click parameter is a list to split it into a string --- workflow/lifecycle/configure.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/workflow/lifecycle/configure.py b/workflow/lifecycle/configure.py index 2856b12..a37b59c 100644 --- a/workflow/lifecycle/configure.py +++ b/workflow/lifecycle/configure.py @@ -101,12 +101,16 @@ def arguments(func: Callable[..., Any], work: Work) -> List[str]: List[str]: List of CLI arguments """ args: List[str] = [] + for parameter in func.params: parameter_name_in_cli = parameter.opts[-1] paraneter_name_in_function = parameter.name parameter_value_in_work = work.parameters.get(paraneter_name_in_function, None) if parameter_value_in_work: + if isinstance(parameter_value_in_work, list): + parameter_value_in_work = " ".join(parameter_value_in_work) + if isinstance(parameter, click.Argument): # If argument, then the parameter is purely positional without a key args.append(f"{parameter_value_in_work}")