-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] Add bytes index primitive #10950
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
Conversation
mistakely deleted an irbuilding test |
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 mostly good, just a few small comments.
Can you also test this with a microbenchmark?
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.
Thanks for the updates! One more comment about tests, otherwise looks good.
b[4] | ||
with assertRaises(IndexError, "index out of range"): | ||
b[-5] | ||
b2 = bytearray([175, 255, 128, 22]) |
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.
Use b2: bytes
here as well.
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.
This still needs to be updated.
Also the Windows 32-bit build is failing. |
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.
One remaining thing, otherwise ready to merge.
b[4] | ||
with assertRaises(IndexError, "index out of range"): | ||
b[-5] | ||
b2 = bytearray([175, 255, 128, 22]) |
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.
This still needs to be updated.
This test case is for |
Ah, fair enough. This is ready to merge then. |
Description
Implements part of mypyc/mypyc#880.
__getitem__
ops for bytes(int
andshort_int
as index parameter)Test Plan
Add several run tests and an irbuild test.