Keyword-only arguments in pipeline function signature break the compilation

[Udiknedormin]

/kind bug

What steps did you take and what happened:
Pass a pipeline function that has keyword-only arguments, e.g. def pipeline(*, keyword_only_argument: str).

Compiler rises:

TypeError: missing a required argument: 'keyword_only_argument'

What did you expect to happen:
Pipeline working just fine, just like for non-keyword-only arguments, e.g. def pipeline(casual_argument: str).

Additional information:
Origins in compiler.py due to using FullArgsSpec.args, which doesn't contain keyword-only arguments:

argspec = inspect.getfullargspec(pipeline_func)
(...)
for arg_name in argspec.args:
  args_list.append(...)

with dsl.Pipeline(pipeline_name) as dsl_pipeline:
  pipeline_func(*args_list)

Environment:

Irrelevant.

[issue-label-bot]

Issue Label Bot is not confident enough to auto-label this issue.
See dashboard for more details.

[ckadner]

So the FullArgSpec does contain the kwonlyargs, which we could make use of here.

class FullArgSpec(NamedTuple):
    args: List[str]
    varargs: Optional[str]
    varkw: Optional[str]
    defaults: Optional[Tuple[Any, ...]]
    kwonlyargs: List[str]
    kwonlydefaults: Optional[Dict[str, Any]]
    annotations: Dict[str, Any]

But is that really a useful thing to do? Why would you define a dsl.Pipeline function with varargs (*) followed by kwonlyargs? Should your pipeline function not have to know about the arguments by name in order to do something useful with them?

[ckadner]

I see Kubeflow changed this code 4 month ago: kubeflow/pipelines@374b3b0

I will create a PR to make that change for the kfp_tekton compiler as well

[issue-label-bot]

Issue-Label Bot is automatically applying the labels:

Label	Probability
area/sdk	0.52

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

[ckadner]

I see Kubeflow changed this code 4 month ago: kubeflow/pipelines@374b3b0

I will create a PR to make that change for the kfp_tekton compiler as well

Fails as well with kfp.Compiler:

keyword_only_pipeline.py:

from kfp import dsl

def echo_op(text):
    return dsl.ContainerOp(name='echo', image='library/bash:4.4.23',
                           command=['sh', '-c'], arguments=['echo "text: $0"', text])

@dsl.pipeline(
    name='kwonlyargs', 
    description='Issue #315'
)
def kwonlyargs_pipeline(*, keyword_only_argument: str):
    echo_task = echo_op(keyword_only_argument)

$ dsl-compile --py keyword_only_pipeline.py --output keyword_only_pipeline.yaml

Traceback (most recent call last):
  File ".venv/bin/dsl-compile", line 8, in <module>
    sys.exit(main())
  File ".venv/lib/python3.7/site-packages/kfp/compiler/main.py", line 104, in main
    not args.disable_type_check,
  File ".venv/lib/python3.7/site-packages/kfp/compiler/main.py", line 91, in compile_pyfile
    _compile_pipeline_function(pipeline_funcs, function_name, output_path, type_check)
  File ".venv/lib/python3.7/site-packages/kfp/compiler/main.py", line 68, in _compile_pipeline_function
    kfp.compiler.Compiler().compile(pipeline_func, output_path, type_check)
  File ".venv/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 905, in compile
    package_path=package_path)
  File ".venv/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 960, in _create_and_write_workflow
    pipeline_conf)
  File ".venv/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 804, in _create_workflow
    pipeline_func(*args_list)
TypeError: kwonlyargs_pipeline() takes 0 positional arguments but 1 was given

So, I go back to my first comment :-)

Why would you define a dsl.Pipeline function with varargs (*) followed by kwonlyargs? Should your pipeline function not have to know about the arguments by name in order to do something useful with them?

[issue-label-bot]

Issue-Label Bot is automatically applying the labels:

Label	Probability
area/sdk-dsl-compiler	0.50

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

[Udiknedormin]

@ckadner

But is that really a useful thing to do? Why would you define a dsl.Pipeline function with varargs (*) followed by kwonlyargs? Should your pipeline function not have to know about the arguments by name in order to do something useful with them?

Default values for arguments are only allowed for keyword-only arguments or last N positional-or-keyword arguments. Keyword-only arguments make it easier to specify default values for arguments.

Fails as well with kfp.Compiler

Yes, I opened the same issue (in progress) to the original kubeflow-pipelines too. Both the solutions and code in kubeflow-pipelines and kfp-tekton were different though, so a change in kubeflow-pipelines wouldn't fix it in kfp-tekton, that's why I opened this issue here.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because it has not had recent activity. Please comment "/reopen" to reopen it.