[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: Closing this; a separate feature request should be opened for the idea of adding __index__ awareness to struct.pack in py3k. -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: I've opened issue 8300 for adding the __index__ handling. -- superseder: - Allow struct.pack to handle objects with an __index__ method. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Meador Inge mead...@gmail.com added the comment: (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! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Florent Xicluna florent.xicl...@gmail.com added the comment: 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. -- nosy: +flox ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: 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. -- Added file: http://bugs.python.org/file16482/issue1530559__int__.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: Updated patch, with slightly saner warnings checks. -- Added file: http://bugs.python.org/file16484/issue1530559__int__2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: Restored use of __int__ for all integer conversion codes in r78762. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: Recent checkins messed up Meador Inge's __index__ patch; here's a regenerated version. -- Added file: http://bugs.python.org/file16485/issue-1530559__index__.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@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 rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Meador Inge mead...@gmail.com added the comment: 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 issue 3132. As that work will be simplified by having the veneer as well. -- nosy: +minge Added file: http://bugs.python.org/file16476/issue-1530559.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Andreas Kloeckner inf...@tiker.net added the comment: 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? -- nosy: +inducer ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: 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. -- status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Andreas Kloeckner inf...@tiker.net added the comment: 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__? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: 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. -- stage: committed/rejected - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: 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. -- priority: normal - high resolution: fixed - ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Andreas Kloeckner inf...@tiker.net added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment: This is now fixed in the trunk in r73858. The failing test has been reenabled. -- resolution: - fixed stage: - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Changes by Daniel Diniz aja...@gmail.com: -- dependencies: +struct.pack(I, foo); struct.pack(L, foo) should fail keywords: +patch type: - behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Changes by Mark Dickinson dicki...@gmail.com: -- assignee: bob.ippolito - marketdickinson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment:
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?
--
priority: critical - normal
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1530559
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Bob Ippolito b...@redivi.com added the comment: I believe that struct.error is just how it worked before 2.5 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment:
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'
--
nosy: +marketdickinson
versions: +Python 2.6, Python 2.7, Python 3.0, Python 3.1 -Python 2.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1530559
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Mark Dickinson dicki...@gmail.com added the comment:
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'
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1530559
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Benjamin Peterson [EMAIL PROTECTED] added the comment: 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. -- nosy: +benjamin.peterson priority: normal - critical ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Changes by Giampaolo Rodola' [EMAIL PROTECTED]: -- nosy: +giampaolo.rodola ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue1530559 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Changes by Sean Reifschneider: -- priority: urgent - normal _ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1530559 _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1530559] struct.pack raises TypeError where it used to convert
Sean Reifschneider added the comment: 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. -- nosy: +jafo _ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1530559 _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
