Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): enable use of primitive placeholders in f-string #8494

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions sdk/python/kfp/components/placeholders.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ def _to_string(self) -> str:
raise NotImplementedError

def __str__(self) -> str:
"""Used for creating readable error messages when a placeholder doesn't
refer to an existing input or output."""
return self.__class__.__name__
"""Enables use of placeholders in f-strings.

To be overridden by container placeholders ConcatPlaceholder and
IfPresentPlaceholder, which cannot be used in an f-string.
"""
return self._to_string()

def __eq__(self, other: Any) -> bool:
"""Used for comparing placeholders in tests."""
Expand Down Expand Up @@ -157,6 +160,10 @@ def _to_dict(self) -> Dict[str, Any]:
def _to_string(self) -> str:
return json.dumps(self._to_dict())

def __str__(self) -> str:
raise ValueError(
f'Cannot use {self.__class__.__name__} in an f-string.')


class IfPresentPlaceholder(Placeholder):
"""Placeholder for handling cases where an input may or may not be passed.
Expand Down Expand Up @@ -256,6 +263,10 @@ def _to_dict(self) -> Dict[str, Any]:
def _to_string(self) -> str:
return json.dumps(self._to_dict())

def __str__(self) -> str:
raise ValueError(
f'Cannot use {self.__class__.__name__} in an f-string.')


_CONTAINER_PLACEHOLDERS = (IfPresentPlaceholder, ConcatPlaceholder)
PRIMITIVE_INPUT_PLACEHOLDERS = (InputValuePlaceholder, InputPathPlaceholder,
Expand Down
73 changes: 73 additions & 0 deletions sdk/python/kfp/components/placeholders_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from kfp import compiler
from kfp import dsl
from kfp.components import placeholders
from kfp.dsl import Artifact
from kfp.dsl import Output


class TestExecutorInputPlaceholder(parameterized.TestCase):
Expand Down Expand Up @@ -396,3 +398,74 @@ def test_primitive_placeholder(self, placeholder: Any, expected: str):
self.assertEqual(
placeholders.convert_command_line_element_to_string_or_struct(
placeholder), expected)


class OtherPlaceholderTests(parameterized.TestCase):

def test_primitive_placeholder_can_be_used_in_fstring1(self):

@dsl.container_component
def echo_bool(boolean: bool = True):
return dsl.ContainerSpec(
image='alpine', command=['sh', '-c', f'echo {boolean}'])

self.assertEqual(
echo_bool.component_spec.implementation.container.command,
['sh', '-c', "echo {{$.inputs.parameters['boolean']}}"])

def test_primitive_placeholder_can_be_used_in_fstring2(self):

@dsl.container_component
def container_with_placeholder_in_fstring(
output_artifact: Output[Artifact],
text1: str,
):
return dsl.ContainerSpec(
image='python:3.7',
command=[
'my_program',
f'prefix-{text1}',
f'{output_artifact.uri}/0',
])

self.assertEqual(
container_with_placeholder_in_fstring.component_spec.implementation
.container.command, [
'my_program',
"prefix-{{$.inputs.parameters['text1']}}",
"{{$.outputs.artifacts['output_artifact'].uri}}/0",
])

def test_cannot_use_concat_placeholder_in_f_string(self):

with self.assertRaisesRegex(
ValueError, 'Cannot use ConcatPlaceholder in an f-string.'):

@dsl.container_component
def container_with_placeholder_in_fstring(
text1: str,
text2: str,
):
return dsl.ContainerSpec(
image='python:3.7',
command=[
'my_program',
f'another-prefix-{dsl.ConcatPlaceholder([text1, text2])}',
])

def test_cannot_use_ifpresent_placeholder_in_f_string(self):

with self.assertRaisesRegex(
ValueError, 'Cannot use IfPresentPlaceholder in an f-string.'):

@dsl.container_component
def container_with_placeholder_in_fstring(
text1: str,
text2: str,
):
return dsl.ContainerSpec(
image='python:3.7',
command=[
'echo',
f"another-prefix-{dsl.IfPresentPlaceholder(input_name='text1', then=['val'])}",
])
6 changes: 3 additions & 3 deletions sdk/python/kfp/components/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,17 +483,17 @@ def check_placeholder_references_valid_io_name(
elif isinstance(arg, placeholders.PRIMITIVE_INPUT_PLACEHOLDERS):
if arg.input_name not in inputs_dict:
raise ValueError(
f'Argument "{arg}" references nonexistant input: "{arg.input_name}".'
f'Argument "{arg.__class__.__name__}" references nonexistant input: "{arg.input_name}".'
)
elif isinstance(arg, placeholders.PRIMITIVE_OUTPUT_PLACEHOLDERS):
if arg.output_name not in outputs_dict:
raise ValueError(
f'Argument "{arg}" references nonexistant output: "{arg.output_name}".'
f'Argument "{arg.__class__.__name__}" references nonexistant output: "{arg.output_name}".'
)
elif isinstance(arg, placeholders.IfPresentPlaceholder):
if arg.input_name not in inputs_dict:
raise ValueError(
f'Argument "{arg}" references nonexistant input: "{arg.input_name}".'
f'Argument "{arg.__class__.__name__}" references nonexistant input: "{arg.input_name}".'
)

all_normalized_args: List[placeholders.CommandLineElement] = []
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright 2022 The Kubeflow Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from kfp import dsl
from kfp.dsl import Artifact
from kfp.dsl import ContainerSpec
from kfp.dsl import Output


@dsl.container_component
def container_with_placeholder_in_fstring(
output_artifact: Output[Artifact],
text1: str = 'text!',
):
return ContainerSpec(
image='python:3.7',
command=[
'my_program',
f'prefix-{text1}',
f'{output_artifact.uri}/0',
])


if __name__ == '__main__':
from kfp import compiler
compiler.Compiler().compile(
pipeline_func=container_with_placeholder_in_fstring,
package_path=__file__.replace('.py', '.yaml'))
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
components:
comp-container-with-placeholder-in-fstring:
executorLabel: exec-container-with-placeholder-in-fstring
inputDefinitions:
parameters:
text1:
defaultValue: text!
parameterType: STRING
outputDefinitions:
artifacts:
output_artifact:
artifactType:
schemaTitle: system.Artifact
schemaVersion: 0.0.1
deploymentSpec:
executors:
exec-container-with-placeholder-in-fstring:
container:
command:
- my_program
- prefix-{{$.inputs.parameters['text1']}}
- '{{$.outputs.artifacts[''output_artifact''].uri}}/0'
image: python:3.7
pipelineInfo:
name: container-with-placeholder-in-fstring
root:
dag:
tasks:
container-with-placeholder-in-fstring:
cachingOptions:
enableCache: true
componentRef:
name: comp-container-with-placeholder-in-fstring
inputs:
parameters:
text1:
componentInputParameter: text1
taskInfo:
name: container-with-placeholder-in-fstring
inputDefinitions:
parameters:
text1:
defaultValue: text!
parameterType: STRING
schemaVersion: 2.1.0
sdkVersion: kfp-2.0.0-beta.6
3 changes: 3 additions & 0 deletions sdk/python/test_data/test_data_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ components:
- module: container_with_if_placeholder
name: container_with_if_placeholder
execute: false
- module: container_with_placeholder_in_fstring
name: container_with_placeholder_in_fstring
execute: false
v1_components:
test_data_dir: sdk/python/test_data/v1_component_yaml
read: true
Expand Down