-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
PEP 3137: make bytesobject.c methods #45602
Comments
This makes all existing bytesobject.c methods use the buffer API rather NOTE: this patch likely depends on http://bugs.python.org/issue1260 |
Patch updated. It now implements the is*() methods for PyBytes. It still TODO: adding the missing methods (listed in a comment in the |
Did you move this into the stringlib subdirectory? That's more for |
Good idea, I haven't done that yet. At the moment it lives in -gps |
Is it worth my time to review this yet? |
here's the latest update. this takes care of all methods listed in the i'm currently working porting some test case code for more methods from |
Same diff 03 here but it also adds unit test coverage for the new |
Very impressive! (Apologies if these lines are occasionally out of order.) +extern PyObject* _bytes_isspace(const char *cptr, const Py_ssize_t len); IMO all these functions should have names starting with _Py or _Py_, Also, why 'const Py_ssize_t'? It's a scalar! +/* these store their len sized answer in the given preallocated *result Mind using a Capital first letter? +extern const char isspace__doc__[]; This needs a _Py or _Py_ prefix too. +extern const unsigned int ctype_table[256]; Now this is no longer static, it also needs a _Py or _Py_ prefix. +extern const unsigned char ctype_tolower[256]; Ditto (the macros are fine though, since they are only visible to code +Return True if all characters in S are whitespace\n\ Shouldn't that be bytes instead of characters (and consistently +/* ------- GPS --------- */ What's this? Your initials? :-) I don't think it's needed. I'm /* The nullbytes are used by the stringlib during partition.
This comment block refers to something that ain't gonna happen + /* XXX: this depends on a signed integer overflow to < 0 */ (And repeated twice more.) Wouldn't it be better to write this in a way +/* TODO(gps): Don't you mean XXX(gps)? :-) + * These methods still need implementing (porting over from Hmmm... Aren't these done now? + /* XXX(gps): the docstring above says any iterable int will do but the Yes, it is, and it's a bug that it currently doesn't. |
Ok, I believe I've fixed everything you commented on, accidentally
< >0 */
This code was copied as is from stringobject.c, I just added the XXX |
04b is the same as 04, i just fixed the docstrings that i had missed in wording could probably be improved further if anyone has any ideas. |
Good! Check it in before I change my mind. :-) The words can be tweaked later.
|
Committed revision 58493 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: