-
Notifications
You must be signed in to change notification settings - Fork 1
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
Introduce a TextTransformer to replace strings with special chars #6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just minor nits.
I'd also love if we'd start documenting the transformer classes as well, but that is a separate action item 😁
tests/test_transformer.py
Outdated
|
||
expected = {"key": "some <value> with more text"} | ||
|
||
# Matching this against a regex does not work, because the + is a special character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: that comment seems like it was referring to an earlier version of the test code here before the parametrization was introduced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was more a generic comment to explain why we're using the TextTransformer in the first place. I've moved this to the TransformerUtility.text
, so all users should now see this explanation:
Creates a new TextTransformer. All occurrences in the string-converted dict will be replaced.
Useful if the text contains special characters that would confuse the RegexTransformer, like '+' or '('.
@@ -211,6 +211,32 @@ def test_nested_sorting_transformer(self): | |||
output = transformer.transform(input, ctx=ctx) | |||
assert output == expected | |||
|
|||
@pytest.mark.parametrize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice test cases 👏
:param text: the text that should be replaced | ||
:param replacement: the value which will replace the original value. | ||
|
||
:return: RegexTransformer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:return: RegexTransformer | |
:return: TextTransformer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed - good catch!
2c93b18
to
87ff322
Compare
The regex transformer works in 99% of the cases, but I ran into some problems when trying to match a base64 representation like
asdf+qwer=
.The regex transformer assumes the
+
is a special character, so it will not match the exact value. Hence the `TextTransformer, that just transforms the entire string as-is.