Skip to content

Commit

Permalink
support primitive placeholders in fstring
Browse files Browse the repository at this point in the history
  • Loading branch information
connor-mccarthy committed Nov 23, 2022
1 parent 4bb57e6 commit ee85d11
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 6 deletions.
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

0 comments on commit ee85d11

Please sign in to comment.