|
|
|
This patch implements the current work in a first step towards implementing the extended struct format string support called for in PEP 3118. Namely, the support for nested structures via 'T{}' is implemented.
Patch Set 1 #
Total comments: 8
Total messages: 2
|
minge
http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c File Modules/_struct.c (right): http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1666 Modules/_struct.c:1666: PyObject *tup = PyTuple_New(0); Since we know the number ...
|
15 years ago (2011年01月07日 03:59:45 UTC) #1 | ||||||||||||||||||||||||||
http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c File Modules/_struct.c (right): http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1666 Modules/_struct.c:1666: PyObject *tup = PyTuple_New(0); Since we know the number of arguments that we are unpacking, we should use 'PyTuple_New(tree->s_len)' here followed by 'PyTuple_SetItem" calls in the loop. Then we can get rid off all the nasty 'PySequence_Concat' calls.
Some comments attached. I wrote some additional things you may wish to put in: (a) Support for the '^' byte order character: https://bitbucket.org/pv/cpython-stuff/changeset/ab9653885bd5 (b) Support for the 'i:fieldname:' field names. They are just skipped, though, but it's probably a good idea to try to support the PEP 3118 syntax as closely as feasible... https://bitbucket.org/pv/cpython-stuff/changeset/e04591f57ccb http://codereview.appspot.com/3863042/diff/1/Doc/library/struct.rst File Doc/library/struct.rst (right): http://codereview.appspot.com/3863042/diff/1/Doc/library/struct.rst#newcode99 Doc/library/struct.rst:99: The "^" code is missing from the table below. http://codereview.appspot.com/3863042/diff/1/Doc/library/struct.rst#newcode152 Doc/library/struct.rst:152: Also "^" does not add padding. http://codereview.appspot.com/3863042/diff/1/Lib/test/test_struct.py File Lib/test/test_struct.py (right): http://codereview.appspot.com/3863042/diff/1/Lib/test/test_struct.py#newcode697 Lib/test/test_struct.py:697: The tests for alignment handling is not fully complete. I wonder if the tests from Numpy's implementation could be ported here; although also that takes some trouble... http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c File Modules/_struct.c (right): http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1301 Modules/_struct.c:1301: return isdigit(c); This probably shouldn't check for isdigit (count => it's not primitive). The count needs only be parsed by `parse_format_string_body`. http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1307 Modules/_struct.c:1307: static char *byte_order_codes = "<>!=@"; What about the '^' byte order marker? It's introduced in pep-3118 as native-endian no-alignment. http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1449 Modules/_struct.c:1449: while (*state->fmt == 'T' || is_primitive(*state->fmt)) { ... || isdigit(*state->fmt) http://codereview.appspot.com/3863042/diff/1/Modules/_struct.c#newcode1824 Modules/_struct.c:1824: n = tree->s_size; Should too long an input string be an error?