piman@toybox:~$ python2.4 -c "import struct;
struct.pack('>H', 1.0)"
piman@toybox:~$ python2.5 -c "import struct;
struct.pack('>H', 1.0)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python2.5/struct.py", line 63, in pack
    return o.pack(*args)
TypeError: unsupported operand type(s) for &: 'float'
and 'long'

This might have appeared as part of the struct
optimizations; if struct isn't going to convert anymore
for performance reasons, I think this should be
mentioned in the release notes. Though personally I
would prefer struct go back to converting its arguments.

Logged In: YES
user_id=849994

Bob?

Logged In: YES
user_id=139309

That wasn't really intentional, but the old behavior looks a bit suspect:

$ python2.4 -c "import struct; print repr(struct.pack('>H', 1.6))"
'\x00\x01'

We could change it to check for floats and do a DeprecationWarning?

Logged In: YES
user_id=849994

I think that's a question for python-dev.

Logged In: YES
user_id=33168

I'd like to see a deprecation warning so old code continues
to work. struct is way to loose and needs to be tightened
up, but that might not fully happen until py3k.

Logged In: YES
user_id=139309

I've attached a patch which should resolve this issue.

Logged In: YES
user_id=33168

This patch (or some variant) was checked in as 51119

Reopening. The test case added in r51119 (specifically, test_1530559() and its infrastructure) doesn't work. The check_float_coerce() function is overwritten by a later assignment, meaning that test_1530559() calls the wrong function. Actually making it call the right check_float_coerce() function results in a NameError ("global name 'func' is not defined").

The reason the test currently passes is because check_float_coerce() is effectively an alias for deprecated_err(). test_1530559() passes deprecated_err() a string as its first argument; when deprecated_err() tries to call the string, a TypeError results. deprecated_err() traps the TypeError and so everything appears from the outside to have gone smoothly.

If the test is tweaked so that struct.pack() is actually invoked with the proper arguments, it fails to raise the expected error on float coercion.

etrepum: Can you fix the test which caused this to be re-opened?

I'm tempted to push this down in priority from urgent to high, since the
code is (apparently) fixed, just the test is failing, but ideally this
could just get fixed.

I ran into this issue converting test_struct to unittest. I would fix
it, but I'm not sure exactly what is intended. I'm raising priority.

Retargeting, now that 2.5 is in security-fix-only mode.

Bob, can you clarify what *should* be happening in 2.6, 2.7, 3.0 and 3.1
for things like:

struct.pack('L', 2009.1)
struct.pack('L', Decimal('3.14')) 

?

It also seems that 'L' and 'Q behave differently. For example, with 2.7:

>>> struct.pack('L', 3.1)
sys:1: DeprecationWarning: integer argument expected, got float
'\x03\x00\x00\x00'
>>> struct.pack('Q', 3.1)
'\x03\x00\x00\x00\x00\x00\x00\x00'

Some more strange results:

Python 2.6.1+ (release26-maint:68613, Jan 15 2009, 15:19:54) 
[GCC 4.0.1 (Apple Inc. build 5490)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from struct import pack
>>> from decimal import Decimal
>>> pack('l', Decimal('3.14'))
'\x03\x00\x00\x00'
>>> pack('L', Decimal('3.14'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for &: 'Decimal' and 'long'

Stealing this from Bob. Bob, please steal it back it you want it!

The deprecated struct features (float coercion, overflow wrapping) have
been removed for py3k in r70497, r70688, r71754. I don't plan to backport
this to 2.7; I'll just try to fix the behaviour in a minimal way there.

One thing that's not clear to me: what's the rationale for raising
struct.error everywhere instead of more specific Python errors; e.g.,
TypeError for struct.pack('L', 'not an integer') and OverflowError for
struct.pack('L', 10**100)? Is there a particular use-case for "except
struct.error"?

I believe that struct.error is just how it worked before 2.5

This is now fixed in the trunk in r73858. The failing test has been
reenabled.

The fix breaks my package PyCUDA, which relies on handing struct something that can be cast to long. (i.e. not a float, but something representing a memory address on a GPU)

Am I out of luck?

I'd be open to re-allowing use of __int__ (and __long__) consistently for all integer packing codes in 2.7, as a temporary measure; I'd really prefer not to allow this for 3.x.

What would make more sense, IMO, would be to allow use of the __index__ method (in both 2.7 and 3.x) to convert custom non-integer classes to integers before packing; this is supposed to be the modern approach to creating integer-like classes that can be used as integers (e.g., in list indices). Andreas, would this work for you, or do you need to be able to use __int__ and/or __long__?

Re-opening while we're discussing this.

AFAICS, __index__ would be fine.

To make sure I understand your proposed solution correctly: You'd go through the argument list beforehand and cast everything that's not a number type or str to int by means of __index__?

More or less, yes: when trying to pack a non-integer x (i.e. something that's not an instance of int or long) with an integer format code (one of 'bBhHiIlLqQ', or 'P'), x.__index__() would be called to convert x to an integer, and that integer would be packed as usual (possibly raising OverflowError). Other format codes wouldn't be affected.

I'm not quite sure what you mean by 'beforehand': the conversion would happen at the same time as it currently does.

It might be a struggle for me to get to this before the 2.7 betas. If anyone's interested in submitting a patch, it would be welcome.

Also, it may be that some of r73858 needs to be reverted; going from accepting non-ints and longs in 2.6 to a TypeError in 2.7 is a bit much; there should have at least been a DeprecationWarning. I need to find some time to look at this properly, and work out what on earth I was thinking at the time.

Thanks for the clarification of 'beforehand'--I had understood from your description that this would be some sort of preprocessing step.

I agree that the TypeError is a bit much, though I'm biased of course. If you'd like my input on things you come up with, please don't hesitate to ping me.

If anyone's interested in submitting a patch, it would be welcome.

Sure. I saw where this was partly addressed in r78690. The attached patch adds support for the '__index__' conversion that Mark
suggested.

At first glance, the patch may seem more than what is needed. However, most of the diffs are due to pulling parts of the C
implementation into a Python veneer. This will make preprocessing work (like what was needs for this fix) easier now and in the future. In addition, it will lay a small amount of groundwork for the changes that are needed for resolving bpo-3132. As that work will be simplified by having the veneer as well.

Here's a patch to restore the old usage of __int__ to convert non-integer arguments; it also produces a DeprecationWarning whenever __int__ is used in this way. For consistency and simplicity, __int__ will be tried for *any* non-integer argument when packing with an integer format; this goes beyond the conversions that 2.6 allows. (In 2.6, the behaviour is somewhat random: it works only for 'bBhHil' in native mode and 'bhil' in non-native mode.)

It doesn't seem worth deliberately trying __long__ as well, so I've left that out. So there's still some possibility for breakage relative to 2.6, when (1) packing using 'Q' or 'q', *and* (2) the object to be packed defines __long__ but not __int__, or defines both __long__ and __int__ in inconsistent ways. The likelihood of (2) seems small enough that this isn't worth worrying about in practice (and the workaround is easy, too).

Andreas, are you in a position to test this patch?

Supporting conversions to integer via __index__ is orthogonal to this; I'll take a look at Meador's patch shortly.

Updated patch, with slightly saner warnings checks.

Restored use of __int__ for all integer conversion codes in r78762.

Recent checkins messed up Meador Inge's __index__ patch; here's a regenerated version.

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 bpo-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.

(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),

I understand. I am worried about that as well. The main area that
concerns me is the interface for the 'Struct' class. I did my best to
match the methods and functions from the C implementation. I need to
look into this in more detail, though. One quirk I currently know
about is that the following methods:

'__delattr__', '__getattribute__', '__setattr__', '__new__'

show up in 'help' for the C implementation, but not the Python one.
Although, the implementations do exist in both places.

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.

Agreed that this is not a really scientific benchmark. I did
experiment with this idea a little further though on my machine (OS X
10.5 32-bit):

==============================================
| Configuration | Seconds |
==============================================
| Original | 1.26 |
----------------------------------------------
| __index__ patch v1 | 1.88 |
----------------------------------------------
| Hoisted imports out of functions | 1.53 |
----------------------------------------------
| Hoisted imports and no __index__ | 1.34 |
----------------------------------------------

So with this simple experiment pulling the 'imports' out of the function
wrappers made quite a difference. And, of course, removing the
'__index__' transform brought the times down even more. So, the
wrapper overhead does not look to be terrible for this simple test.

However, as you alluded to, we should find a better benchmark. Any
ideas on a good one?

(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.

Agreed. Thanks for taking the time to look at the patch anyway!

I would suggest to raise a py3k warning instead of a plain warning.
AFAIU the implicit conversion is OK in 2.7, and it is removed in 3.x.

Closing this; a separate feature request should be opened for the idea of adding __index__ awareness to struct.pack in py3k.

I've opened bpo-8300 for adding the __index__ handling.