[mypyc] Optimize str.startswith and str.endswith with tuple argument#18678
[mypyc] Optimize str.startswith and str.endswith with tuple argument#18678JukkaL merged 3 commits intopython:masterfrom
Conversation
d25cb2b to
1ecfacc
Compare
|
|
JukkaL
left a comment
There was a problem hiding this comment.
Looks good, just some ideas about additional test coverage.
| 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.
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.
bb479ed to
5ab9840
Compare
| from typing import Tuple | ||
|
|
||
| def do_startswith(s1: str, s2: Tuple[str, ...]) -> bool: | ||
| return s1.startswith(s2) |
There was a problem hiding this comment.
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.
JukkaL
left a comment
There was a problem hiding this comment.
Thanks! We can improve this later by avoiding tuple boxing, but this is already a nice improvement.
…ython#18703) Followup to python#18678 Missed that we can also use `bool_rprimitive` as return type with a value of `2` for errors.
No description provided.