[mypyc] feat: new primitives for bytes.rjust and bytes.ljust
#19672
Conversation
BobTheBuidler
force-pushed
the
just
branch
2 times, most recently
from
14b314e to
f766dff
Compare
|
|
||
|
|
||
| PyObject *CPyBytes_RjustCustomFill(PyObject *self, CPyTagged width, PyObject *fillbyte) { | ||
| if (!PyBytes_Check(self)) { |
There was a problem hiding this comment.
Can we get rid of these type checks?
There was a problem hiding this comment.
A variable annotated with bytes can also be a bytearray, and all primitives need to support both. Also we need to consider subclasses overriding the method -- we only assume a small number of key methods are not overridden, and we try to keep this set small. These methods are not common enough to be worth adding as special cases, when more commonly used methods support overriding. See CPyBytes_Join for an example of how to do this -- special case for exact bytes values (i.e. no subclass instances) and provide a fallback implementation using generic operations.
| PyErr_SetString(PyExc_TypeError, "self must be bytes"); | ||
| return NULL; | ||
| } | ||
| if (!PyBytes_Check(fillbyte) || PyBytes_Size(fillbyte) != 1) { |
There was a problem hiding this comment.
Can we get rid of these type checks?
There was a problem hiding this comment.
See above -- we need type checks, and they must use an exact bytes check, and there must also be a fallback implementation.
|
|
||
|
|
||
| PyObject *CPyBytes_RjustCustomFill(PyObject *self, CPyTagged width, PyObject *fillbyte) { | ||
| if (!PyBytes_Check(self)) { |
There was a problem hiding this comment.
A variable annotated with bytes can also be a bytearray, and all primitives need to support both. Also we need to consider subclasses overriding the method -- we only assume a small number of key methods are not overridden, and we try to keep this set small. These methods are not common enough to be worth adding as special cases, when more commonly used methods support overriding. See CPyBytes_Join for an example of how to do this -- special case for exact bytes values (i.e. no subclass instances) and provide a fallback implementation using generic operations.
| PyErr_SetString(PyExc_TypeError, "self must be bytes"); | ||
| return NULL; | ||
| } | ||
| if (!PyBytes_Check(fillbyte) || PyBytes_Size(fillbyte) != 1) { |
There was a problem hiding this comment.
See above -- we need type checks, and they must use an exact bytes check, and there must also be a fallback implementation.
| char *res_buf = PyBytes_AsString(result); | ||
| memset(res_buf, ' ', pad); | ||
| memcpy(res_buf + pad, PyBytes_AsString(self), len); | ||
| return result; |
There was a problem hiding this comment.
Please share most of the implementations using a helper function, since the functions are very similar (across all four functions), and these methods probably aren't super performance critical to make the code duplication worth it (the likely gains would be very small, since most of the CPU is spent in the common shared code). E.g. add a function that takes the bytes value, width, a bool flag for ljust/rjust and the fill character (char) as arguments.
2 participants
This PR adds primitives for:
bytes.rjust(width)bytes.ljust(width, fillbyte)bytes.rjust(width)bytes.ljust(width, fillbyte)This PR is ready for review.