-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[mypyc] Add primitive ops for dictionary methods #8742
Conversation
...and as usual Python 3.5 is totally busted. I will probably just disable this on Python 3.5. PR #8725 adds the corresponding |
@msullivan @JukkaL I switched to disabling the optimization for subclasses here too. Now tests pass on Python 3.5. |
I measured that this can make the ops at least 2x faster for small dictionaries. This is a great improvement! |
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.
Looks great, and the performance improvement is also significant. Left some ideas about additional tests.
@@ -1035,14 +1035,17 @@ def u(x: int) -> int: | |||
d.update(x=x) | |||
return d['x'] | |||
|
|||
def get_content(d: Dict[int, int]) -> Tuple[List[int], List[int], List[Tuple[int, int]]]: | |||
return list(d.keys()), list(d.values()), list(d.items()) |
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.
Add also tests for the "plain" primitive ops that don't create a list, such as by doing set(d.keys())
?
Might be worth testing dict subclasses as well.
This covers both view and list versions (it looks like both are used relatively often, at least in mypy). This accompanies #8725 to account for cases where
keys
/values
/items
appear in non-loop contexts.