Allow struct.pack to handle objects with an __index__ method.

[mdickinson]

BPO	8300
Nosy	@mdickinson, @meadori, @anacrolix
Files	struct_index_trunk.patch struct_index_trunk2.patch struct_index_trunk3.patch

^{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:

assignee = 'https://github.com/mdickinson'
closed_at = <Date 2010-04-05.07:47:44.833>
created_at = <Date 2010-04-03.11:51:46.746>
labels = ['extension-modules', 'type-feature']
title = 'Allow struct.pack to handle objects with an __index__ method.'
updated_at = <Date 2011-02-20.04:14:38.102>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2011-02-20.04:14:38.102>
actor = 'anacrolix'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2010-04-05.07:47:44.833>
closer = 'mark.dickinson'
components = ['Extension Modules']
creation = <Date 2010-04-03.11:51:46.746>
creator = 'mark.dickinson'
dependencies = []
files = ['16746', '16748', '16751']
hgrepos = []
issue_num = 8300
keywords = ['patch']
message_count = 14.0
messages = ['102245', '102257', '102258', '102262', '102263', '102311', '102325', '102326', '102327', '102361', '102364', '128849', '128877', '128891']
nosy_count = 3.0
nosy_names = ['mark.dickinson', 'meador.inge', 'anacrolix']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8300'
versions = ['Python 2.7', 'Python 3.2']

[mdickinson]

In Python 2.7, struct.pack with an integer format can handle non-integers that provide an __int__ method (although this *does* raise a DeprecationWarning).

Python 2.7a4+ (trunk:79659:79661, Apr  3 2010, 11:28:19) 
[GCC 4.2.1 (Apple Inc. build 5646) (dot 1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from struct import pack
[35194 refs]
>>> pack('L', 3.1415)
'\x03\x00\x00\x00\x00\x00\x00\x00'
[35210 refs]

This behaviour isn't particularly desirable for floats or Decimal instances, but it's useful for integer-like objects.

In Python 3.x, there's no provision for handling integer-like objects than aren't actually integers.

I propose that in 3.x, struct.pack should try to convert any non-integer to an integer by using its __index__ method, before packing.

[mdickinson]

Here's a patch for trunk. It combines the docs and tests from Meador Inge's patch in bpo-1530559 with a C-level change to get_pylong in Modules/struct.c.

[mdickinson]

Adding Meador Inge to nosy.

[mdickinson]

That patch was a bit hasty in many respects; here's a better one.

For 2.7, the scheme is as follows: when packing a non-integer with an integer format:

(1) First __index__ is tried
(2) If the __index__ method doesn't exist, or the call to __index__ raises TypeError, then the __int__ method is tried.
(3) If the __index__ method raises something other than TypeError, or returns a non-integer, then struct.pack fails.

[mdickinson]

Committed this patch to trunk in r79674. Will forward port to py3k.

[meadori]

I may be missing something subtle, but how can 'PyNumber_Index(v) != NULL' *and* '!PyInt_Check(v) && !PyLong_Check(v)' both be satisfied in the 'get_pylong' mods? It seems to me that 'PyNumber_Index' only returns non-NULL when the object being returned is an 'int' or 'long'.

Attached a patch with the extra check removed and a few more test cases.

[mdickinson]

Probably both those conditions can't be satisfied; I'm wasn't sure what happened if something's __index__ method returned something other than an int or long.

But now I bother to look at the source (in Objects/abstract.c) I see that there *is* already an explicit check for the result of nb_index being int or long (with TypeError being raised if the result isn't one of those). Mea culpa. I'll remove those lines (though I may leave an assert, just to be on the safe side).

The 2.x behaviour isn't ideal: I'd prefer to just stop if the __index__ method is present and raises TypeError, rather than going on to check __int__ in that case. But that presents problems with old-style classes, where PyIndex_Check is true even when no __index__ method is explicitly defined.

Thanks for the extra tests!

[mdickinson]

Committed (with some tabs in test_struct.py changed to spaces) to trunk in r79745.

[mdickinson]

Merged to py3k in r79746.

Meador, does this all look okay, now?

[meadori]

Looks good to me.

[mdickinson]

Thanks.

[anacrolix]

Why isn't this implemented to work with __int__ as well?

[mdickinson]

Because (arguably) we don't want to be able to pack non-integral floats (or Decimal instances, or ...) using integer formats:

>>> import struct
[56090 refs]
>>> struct.pack('L', 2.3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: required argument is not an integer
[56125 refs]

[anacrolix]

Thanks Mark for clearing that up. I found this link to be useful in explaining the purpose of __index__: http://docs.python.org/release/2.5.1/whatsnew/pep-357.html

I think the choice of allowing only __index__ was the right choice.