classification
Title: Allow struct.pack to handle objects with an __index__ method.
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: anacrolix, mark.dickinson, meador.inge
Priority: normal Keywords: patch

Created on 2010-04-03 11:51 by mark.dickinson, last changed 2011-02-20 04:14 by anacrolix. This issue is now closed.

Files
File name Uploaded Description Edit
struct_index_trunk.patch mark.dickinson, 2010-04-03 13:24
struct_index_trunk2.patch mark.dickinson, 2010-04-03 13:57
struct_index_trunk3.patch meador.inge, 2010-04-04 00:38
Messages (14)
msg102245 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 11:51
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.
msg102257 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 13:24
Here's a patch for trunk.  It combines the docs and tests from Meador Inge's patch in issue 1530559 with a C-level change to get_pylong in Modules/struct.c.
msg102258 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 13:25
Adding Meador Inge to nosy.
msg102262 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 13:57
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.
msg102263 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 14:06
Committed this patch to trunk in r79674.  Will forward port to py3k.
msg102311 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-04-04 00:38
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.
msg102325 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-04 08:34
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!
msg102326 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-04 08:44
Committed (with some tabs in test_struct.py changed to spaces) to trunk in r79745.
msg102327 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-04 08:53
Merged to py3k in r79746.

Meador, does this all look okay, now?
msg102361 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-04-05 03:16
Looks good to me.
msg102364 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-05 07:47
Thanks.
msg128849 - (view) Author: Matt Joiner (anacrolix) Date: 2011-02-19 15:30
Why isn't this implemented to work with __int__ as well?
msg128877 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-02-19 19:07
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]
msg128891 - (view) Author: Matt Joiner (anacrolix) Date: 2011-02-20 04:14
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.
History
Date User Action Args
2011-02-20 04:14:38anacrolixsetmessages: + msg128891
2011-02-19 19:07:51mark.dickinsonsetmessages: + msg128877
2011-02-19 15:30:00anacrolixsetnosy: + anacrolix
messages: + msg128849
2010-04-05 07:47:44mark.dickinsonsetstatus: open -> closed

messages: + msg102364
2010-04-05 03:16:38meador.ingesetstatus: pending -> open

messages: + msg102361
2010-04-04 08:53:59mark.dickinsonsetstatus: open -> pending
resolution: accepted
messages: + msg102327

stage: patch review -> resolved
2010-04-04 08:44:16mark.dickinsonsetmessages: + msg102326
2010-04-04 08:34:32mark.dickinsonsetmessages: + msg102325
2010-04-04 00:38:06meador.ingesetfiles: + struct_index_trunk3.patch

messages: + msg102311
2010-04-03 14:06:27mark.dickinsonsetmessages: + msg102263
2010-04-03 13:58:52mark.dickinsonsetpriority: normal
stage: test needed -> patch review
2010-04-03 13:57:07mark.dickinsonsetfiles: + struct_index_trunk2.patch

messages: + msg102262
2010-04-03 13:25:09mark.dickinsonsetnosy: + meador.inge
messages: + msg102258
2010-04-03 13:24:13mark.dickinsonsetfiles: + struct_index_trunk.patch
keywords: + patch
messages: + msg102257

versions: + Python 2.7
2010-04-03 11:52:52mark.dickinsonlinkissue1530559 superseder
2010-04-03 11:52:08mark.dickinsonsettype: enhancement
components: + Extension Modules
stage: test needed
2010-04-03 11:51:46mark.dickinsoncreate