Skip to content

Commit 810e466

Browse files
committed
Raise an error when a placeholder is used in an f-string or .format() call.
This is a common mistake that can lead to confusing errors, because when combined with surrounding strings, it can lead to invalid values like `--data_path=input:<tfx.dsl.placeholder.placeholder.ArtifactPlaceholder\ object\ at\ 0x7f42d66671f0>/filename.txt`. The component implementation might receive as an argument in a place where it normally expects a proper file path, causing it to print weird errors or even read an empty file instead, depending on its implementation. Note: If you actually intend to print the placeholder to the developer/console, use `repr()` instead. PiperOrigin-RevId: 640889882
1 parent baab834 commit 810e466

File tree

4 files changed

+16
-2
lines changed

4 files changed

+16
-2
lines changed

RELEASE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
## Breaking Changes
66

7+
* `Placeholder.__format__()` is now disallowed, so you cannot use placeholders
8+
in f-strings and `str.format()` calls anymore. If you get an error from this,
9+
most likely you discovered a bug and should not use an f-string in the first
10+
place. If it is truly your intention to print the placeholder (not its
11+
resolved value) for debugging purposes, use `repr()` or `!r` instead.
12+
713
### For Pipeline Authors
814

915
### For Component Authors

tfx/dsl/experimental/conditionals/conditional.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def validate(self, containing_nodes: Sequence[base_node.BaseNode]):
3636
if id(ancestor_context.predicate) == id(self.predicate):
3737
raise ValueError(
3838
'Nested conditionals with duplicate predicates:\n'
39-
f'{self.predicate} vs\n{ancestor_context.predicate}.\n'
39+
f'{self.predicate!r} vs\n{ancestor_context.predicate!r}.\n'
4040
'Please merge the redundant conditionals.'
4141
)
4242

tfx/dsl/placeholder/placeholder_base.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,14 @@ def __iter__(self) -> Iterator[Any]:
145145
'Did you miss the ending `,` in your tuple?'
146146
)
147147

148+
def __format__(self, format_spec) -> str:
149+
raise RuntimeError(
150+
'Formatting a placeholder is not supported. Did you accidentally use a '
151+
'placeholder inside an f-string or .format() call? That cannot work '
152+
'because placeholder values are only known later at runtime. You can '
153+
'use the + operator for string concatenation.'
154+
)
155+
148156
def b64encode(self, url_safe: bool = True) -> _Base64EncodeOperator:
149157
"""Encodes the value with URL-safe Base64 encoding."""
150158
return _Base64EncodeOperator(self, url_safe)

tfx/types/channel_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ def unwrap_simple_channel_placeholder(
248248
):
249249
raise ValueError(
250250
'Expected placeholder of shape somechannel.future()[0].value, but got'
251-
f' {placeholder}.'
251+
f' {placeholder!r}.'
252252
)
253253

254254
# Now that we know there's only one channel inside, we can just extract it:

0 commit comments

Comments
 (0)