[issue1530559] struct.pack raises TypeError where it used to convert

[Mark Dickinson]

Mark Dickinson <dickinsm at gmail.com> added the comment:

Comments and thoughts on the __index__ patch:

(1) Thank you for a remarkably complete patch!

(2) For 2.x, I'm a bit uncomfortable with introducing the extra Python layer on top of the C layer.  Partly I'm worried about accidentally breaking something (it's not 100% clear to me whether there might be hidden side-effects to such a change), but I also notice that this seems to have a significant impact on performance.  In fact, I seem to recall that the previously existing Python component of the struct module was absorbed into Modules/_struct.c precisely for performance reasons.

A quick, unscientific benchmark:  the time taken to run test_struct with this patch (excluding the changes to test_struct itself) on my machine (OS X 10.6, 64-bit non-framework non-debug build of Python) is around 1.52--1.53 seconds;  without the patch it's around 1.02--1.03 seconds.

(3) For 3.x, and for the issue 3132 work, I agree it might make sense to have a fatter Python layer;  this would also help other Python implementations that are trying to keep up with CPython.  I'm still a bit worried about performance, though.

(4) For 2.x, perhaps we don't need the extra __index__ functionality anyway, now that the previous use of __int__ has been restored.  That would give us a bit more time to think about this for 3.x.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue1530559>
_______________________________________