Skip to content

Commit 3170e5c

Browse files
huaiyanRobben Wang
and
Robben Wang
authored
Filter out None for exceptions's message_parameter (#680)
`string.Formatter().parse(message_format)` will generate None in field_name. Filter it out since it may introduce issue for jsonify. Another option is using regular expressions, Here's a comparison of both options from gpt: Using string.Formatter().parse(): Pros: - The built-in string.Formatter().parse() method is designed specifically for parsing format strings, so it is likely to handle edge cases correctly. - This method doesn't require an additional import. Cons: - The code might be less readable for people who are not familiar with the string.Formatter() class and its .parse() method. - It requires an additional function (iter_field_name()) and a loop to filter out the None values. Using re (regular expressions): Pros: - The code is more concise and easier to read for those familiar with regular expressions. - It requires fewer lines of code and directly extracts the field names, so no additional function or loop is needed for filtering None values. Cons: - Regular expressions might not catch all edge cases specific to format strings (although it will work correctly for most common cases). - People not familiar with regular expressions might find this approach less readable. In summary, both methods will work for most common cases. However, if you are mainly concerned with readability and conciseness, the regular expression method is a better choice. On the other hand, if you prioritize **handling all edge cases related to format strings**, the string.Formatter().parse() method is the safer option. Related PR: #642 # All Promptflow Contribution checklist: - [X] **The pull request does not introduce [breaking changes].** - [ ] **CHANGELOG is updated for new features, bug fixes or other significant changes.** - [X] **I have read the [contribution guidelines](../CONTRIBUTING.md).** - [ ] **Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: [suggested workflow](../CONTRIBUTING.md#suggested-workflow).** ## General Guidelines and Best Practices - [X] Title of the pull request is clear and informative. - [X] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md). ### Testing Guidelines - [ ] Pull request includes test coverage for the included changes. --------- Co-authored-by: Robben Wang <[email protected]>
1 parent aafdf4e commit 3170e5c

File tree

2 files changed

+4
-1
lines changed

2 files changed

+4
-1
lines changed

src/promptflow/promptflow/exceptions.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ def iter_field_name():
180180
return
181181

182182
for _, field_name, _, _ in string.Formatter().parse(message_format):
183-
yield field_name
183+
# Last one field_name is always None, filter it out to avoid exception.
184+
if field_name is not None:
185+
yield field_name
184186

185187
# Use set to remove duplicates
186188
return set(iter_field_name())

src/promptflow/tests/executor/unittests/executor/test_exceptions.py

+1
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ def test_exception_message(self):
1212
)
1313

1414
assert ex.message == "Test exception message with parameters: test_param, <param1>."
15+
assert None not in ex.message_parameters

0 commit comments

Comments
 (0)