-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[mypyc] Optimize str.startswith and str.endswith with tuple argument #18678
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,12 +20,15 @@ def eq(x: str) -> int: | |
| return 2 | ||
| def match(x: str, y: str) -> Tuple[bool, bool]: | ||
| return (x.startswith(y), x.endswith(y)) | ||
| def match_tuple(x: str, y: Tuple[str, ...]) -> Tuple[bool, bool]: | ||
| return (x.startswith(y), x.endswith(y)) | ||
| def remove_prefix_suffix(x: str, y: str) -> Tuple[str, str]: | ||
| return (x.removeprefix(y), x.removesuffix(y)) | ||
|
|
||
| [file driver.py] | ||
| from native import f, g, tostr, booltostr, concat, eq, match, remove_prefix_suffix | ||
| from native import f, g, tostr, booltostr, concat, eq, match, match_tuple, remove_prefix_suffix | ||
| import sys | ||
| from testutil import assertRaises | ||
|
|
||
| assert f() == 'some string' | ||
| assert f() is sys.intern('some string') | ||
|
|
@@ -45,6 +48,16 @@ assert match('abc', '') == (True, True) | |
| assert match('abc', 'a') == (True, False) | ||
| assert match('abc', 'c') == (False, True) | ||
| assert match('', 'abc') == (False, False) | ||
| assert match_tuple('abc', ('d', 'e')) == (False, False) | ||
| assert match_tuple('abc', ('a', 'c')) == (True, True) | ||
| assert match_tuple('abc', ('a',)) == (True, False) | ||
| assert match_tuple('abc', ('c',)) == (False, True) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test case where startswith matches a non-first tuple item. Also add test for error case (tuple contains a non-string). It would be good to add an irbuild test for a tuple literal argument, since it's easy to imagine how a fixed-length tuple literal wouldn't match against a variable-length tuple in the primitive arg type. |
||
| assert match_tuple('abc', ('x', 'y', 'z')) == (False, False) | ||
| assert match_tuple('abc', ('x', 'y', 'z', 'a', 'c')) == (True, True) | ||
| with assertRaises(TypeError, "tuple for startswith must only contain str"): | ||
| assert match_tuple('abc', (None,)) | ||
| with assertRaises(TypeError, "tuple for endswith must only contain str"): | ||
| assert match_tuple('abc', ('a', None)) | ||
|
|
||
| assert remove_prefix_suffix('', '') == ('', '') | ||
| assert remove_prefix_suffix('abc', 'a') == ('bc', 'abc') | ||
|
|
||
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.
I'm more interested in what happens when
startswithis given a tuple literal argument (e.g.return s1.startswith(('x', 'y'))), since this is probably the most common use case. Can you test also this? It would be good to have also a run test for this.