-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[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 all 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,20 @@ 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 match_tuple_literal_args(x: str, y: str, z: str) -> Tuple[bool, bool]: | ||
return (x.startswith((y, z)), x.endswith((y, z))) | ||
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, | ||
match_tuple_literal_args, remove_prefix_suffix | ||
) | ||
import sys | ||
from testutil import assertRaises | ||
|
||
assert f() == 'some string' | ||
assert f() is sys.intern('some string') | ||
|
@@ -45,6 +53,18 @@ 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) | ||
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 match_tuple_literal_args('abc', 'z', 'a') == (True, False) | ||
assert match_tuple_literal_args('abc', 'z', 'c') == (False, True) | ||
|
||
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
startswith
is 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.