classification
Title: long long support for array module
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: cmcqueen1975, haypo, mark.dickinson, mattchaput, meador.inge, ocean-city, orenti, python-dev, skrah
Priority: normal Keywords: patch

Created on 2005-03-29 18:58 by orenti, last changed 2011-09-21 08:14 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
arraymodule-longlong.diff orenti, 2005-03-29 18:58
arraymodule-longlong-2.5.diff orenti, 2005-04-07 23:53
array_long_long.patch ocean-city, 2009-07-03 14:17 for trunk review
issue-1172711.patch meador.inge, 2010-08-24 13:09 py3k patch review
issue-1172711.patch meador.inge, 2011-09-05 02:25 Patch against tip (3.3.0a0) review
issue-1172711.patch meador.inge, 2011-09-09 03:02 v2 patch against tip (3.3.0a0) review
issue-1172711.patch meador.inge, 2011-09-14 01:20 v3 patch against tip (3.3.0a0) review
Messages (27)
msg48085 - (view) Author: Oren Tirosh (orenti) Date: 2005-03-29 18:58
This patch adds signed and unsigned long long support
to the array module. These types are already supported
by the struct module and use the same format characters
(q/Q).

Also corrects a minor bug in PyLong_AsUnsignedLongLong
which reports a BadInternalCall for arguments of
inappropriate type rather than a mere TypeError as
reported by the other conversion functions.
msg48086 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-04-03 14:24
Logged In: YES 
user_id=4771

No two conversion function in longobject.c seem to have the same rules for what to do about non-long objects :-(  I'm afraid some clean-up would be useful, but also difficult for fear of breaking existing user C code :-(

In fact, your patch doesn't apply with today's CVS because someone already tried to add some magic in PyObject_AsLongLong().  It also fails on test_array.py and applies uncleanly on arraymodule.c.

Also, it needs to update the array module documentation.
msg48087 - (view) Author: Oren Tirosh (orenti) Date: 2005-04-07 23:53
Logged In: YES 
user_id=562624

My patch was against 2.4... (duh!)

The other bug is already fixed on 2.5.
msg90059 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2009-07-03 14:07
How about this patch? I haven't tested so intensely, but testcase seems
working.
msg108500 - (view) Author: Craig McQueen (cmcqueen1975) Date: 2010-06-24 04:46
So it looks as though this isn't going in to Python 2.7.

How about 3.x?
msg108508 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-06-24 12:21
Seems like a reasonable addition to me.  Anyone feel like refreshing the patch so that it applies to py3k?
msg108509 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-06-24 12:25
BTW, the PyLong_AsUnsignedLongLong BadInternalCall has long since disappeared.  I agree with Armin Rigo that the conversion functions in longobject.c are a mess, though (and also that cleanup is difficult).
msg114784 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-08-24 13:09
Overall the patch looks good.  I don't think it is an extremely important feature, but similar support is already available in other places (e.g. 'struct', 'ctypes').

Here is a patch updated for py3k with some minor additions:

   (1) Fixed some doc inconsistencies.
   (2) Added pickling support for the new type codes.  The special
       pickling support looks only to be in py3k.

(2) needs unit tests if possible.  If anyone has any good ideas on how to test, then I would be happy to implement the tests.
msg126142 - (view) Author: Matt Chaput (mattchaput) Date: 2011-01-12 21:31
This is an important feature to me. Now I get to redo a bunch of code to have two completely different code paths to do the same thing because nobody could be bothered to keep array up-to-date.
msg143507 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-05 02:25
Here is a refresh of this patch for 3.3.  Please review.
msg143745 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-09-08 23:28
+if have_long_long:
+    class LongLongTest(SignedNumberTest):
+        ...

It is maybe better to use @unittest.skipIf(not have_long_long, 'need long long support'). Except of this nit, the patch looks correct.
msg143751 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-09 03:04
Victor, I like the decorator approach much better.  Thanks.  Attached is a new patch with that update.
msg143856 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-09-11 08:57
I left some remarks on Rietveld.
msg143934 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-09-12 19:55
I made the observation on Rietveld that the following code is never
executed by the test suite. The same applies to similar existing
passages in arraymodule.c:

http://bugs.python.org/review/1172711/diff/3310/10310#newcode394


Meador correctly pointed out that the code allows for duck typing.
But the struct module (and by extension memoryview that must follow
the struct module) don't:

>>> import array, struct
>>> a = array.array('L', [1,2,3])
>>> class T(object):
...     def __init__(self, value):
...         self.value = value
...     def __int__(self):
...          return self.value
...
>>> a = array.array('L', [1,2,3])
>>> struct.pack_into('L', a, 0, 9)
>>> a
array('L', [9, 2, 3])
>>> a[0] = T(100)
>>> a
array('L', [100, 2, 3])
>>> struct.pack_into('L', a, 0, T(200))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: required argument is not an integer
>>>

I vastly prefer the struct module behavior. Since the code isn't executed
by any tests:

Is it really the intention for array to allow duck typing? The documentation
says:

"This module defines an object type which can compactly represent an array
 of basic values: characters, integers, floating point numbers."

"Basic value" doesn't sound to me like "anything that has an __int__() method".


Also, consider this:

>>> sum([T(1),T(2),T(3)])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'T'

>>> sum(array.array('L', [T(1),T(2),T(3)]))
6
msg143948 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-13 02:15
>>>> import array, struct
>>>> a = array.array('L', [1,2,3])
>>>> class T(object):
> ...     def __init__(self, value):
> ...         self.value = value
> ...     def __int__(self):
> ...          return self.value
> ...
>>>> a = array.array('L', [1,2,3])
>>>> struct.pack_into('L', a, 0, 9)
>>>> a
> array('L', [9, 2, 3])
>>>> a[0] = T(100)
>>>> a
> array('L', [100, 2, 3])
>>>> struct.pack_into('L', a, 0, T(200))
> Traceback (most recent call last):
>  File "<stdin>", line 1, in <module>
> struct.error: required argument is not an integer
>>>>
>
> I vastly prefer the struct module behavior. Since the code isn't executed
> by any tests:

Yeah, but if it is a good feature we can always add more tests.  I think the
real issue is whether or not this behavior is even desirable.  Also, similar
behavior can be achieved with struct by using '__index__':

...      def __init__(self, value):
...          self.value = value
...      def __int__(self):
...           return self.value
...      def __index__(self):
...           return self.value
...
>>> a = array.array('L', [1,2,3])
>>> struct.pack_into('L', a, 0, T(200))
>>> a
array('L', [200, 2, 3])

Also, check out issue1530559.  Originally, struct did allow the
'__int__' and '__long__' behavior, but it was deprecated and replaced
with '__index__'.  Maybe we should do the same for array?

IMO, having some way to convert objects to integers is a nice feature
and I think we will find more cases like the PyCUDA case from
issue1530559 where folks need this.
msg143955 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-09-13 11:45
Yes, please let's not add any new __int__-based duck typing here; IMO, we should be moving away from such uses of __int__.  I'd be fine with __index__ based duck-typing.
msg143980 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-13 17:52
> Yes, please let's not add any new __int__-based duck typing here;

Mark, just to clarify a bit, the behavior is already there in the array module (by way of 'PyLong_AsLong').  The fact that it is there was picked up on a code review for this issue.

Anyway, I think we should open a new issue to track the '__index__' vs. '__int__' stuff.
msg143983 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-09-13 19:03
> Mark, just to clarify a bit, the behavior is already there in the array module

Okay, understood.  But the new 'long long' support provided by this patch still allows for __int__-based duck typing, right?

>>> array('Q', [1, 2, Decimal(3.2)])
array('Q', [1, 2, 3])

That's the new duck typing I meant.  I see this acceptance of things with an __int__ method as a mistake, and my gut reaction earlier was that it seems wrong to propagate that mistake into the new long long functionality, even though it's already present in other places in the array module.

On second thoughts though, it would be a peculiar inconsistency to be able to pass Decimal objects to array('L', ...) but not to array('Q', ...).  So probably better to accept this behaviour for now, and open another issue for the __int__ / __index__ discussion, as you suggest.

BTW, the patch and tests look good to me, and all tests pass here (OS X !0.6, 64-bit) (Well, not quite true, but I fail to see how these changes could be responsible for the test_socket and test_packaging failures I'm seeing :-).  I get compile-time warnings from the 'int' declarations that should be 'Py_ssize_t', but I understand that's taken care of already...
msg144001 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-14 00:56
> Okay, understood.  But the new 'long long' support provided by this patch still allows for __int__-based duck typing, right?

Yes, but ...

> That's the new duck typing I meant.  I see this acceptance of things with an __int__ method as a mistake, and my gut
> reaction earlier was that it seems wrong to propagate that mistake into the new long long functionality, even though it's already
> present in other places in the array module.
>
> On second thoughts though, it would be a peculiar inconsistency to be able to pass Decimal objects to array('L', ...) but not
> to array('Q', ...).  So probably better to accept this behaviour for now, and open another issue for the __int__ / __index__ discussion,
> as you suggest.

... I had this inconsistency in mind.  I opened issue12974 for the
__int__/__index__ problem.

Now we just have to figure out which issue gets fixed first :-D  I am
OK with applying the fix for this issue first.
msg144003 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-14 01:20
Updated patch with the 'Py_ssize_t' fixes.
msg144062 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-09-15 01:12
@meadori: please write the version of your patch directly in the filename. For example, I use the pattern: name.patch, name-2.patch, name-3.patch, ...
msg144121 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-09-16 10:34
> I am OK with applying the fix for this issue first.

I also think this should be committed first.
msg144125 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-09-16 11:55
Agreed.  Commit first;  worry about __int__ and __index__ later.
msg144359 - (view) Author: Roundup Robot (python-dev) Date: 2011-09-21 01:04
New changeset 15659e0e2b2e by Meador Inge in branch 'default':
Issue #1172711: Add 'long long' support to the array module.
http://hg.python.org/cpython/rev/15659e0e2b2e
msg144360 - (view) Author: Roundup Robot (python-dev) Date: 2011-09-21 01:20
New changeset 3c56e546dc60 by Victor Stinner in branch 'default':
Issue #1172711: Update What's New in Python 3.3 document for the struct module
http://hg.python.org/cpython/rev/3c56e546dc60
msg144364 - (view) Author: Roundup Robot (python-dev) Date: 2011-09-21 02:49
New changeset 672b63aff0f4 by Meador Inge in branch 'default':
Issue #1172711: Update What's New in Python 3.3 document for the array module.
http://hg.python.org/cpython/rev/672b63aff0f4
msg144371 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-09-21 08:14
> New changeset 672b63aff0f4 by Meador Inge in branch 'default'

Woops, I wrote the wrong module name. Thanks for fixing it.
History
Date User Action Args
2011-09-21 08:14:34hayposetmessages: + msg144371
2011-09-21 02:49:27python-devsetmessages: + msg144364
2011-09-21 01:20:08python-devsetmessages: + msg144360
2011-09-21 01:06:14meador.ingesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2011-09-21 01:04:06python-devsetnosy: + python-dev
messages: + msg144359
2011-09-16 11:55:53mark.dickinsonsetmessages: + msg144125
2011-09-16 10:34:43skrahsetmessages: + msg144121
2011-09-15 01:12:10hayposetmessages: + msg144062
2011-09-14 01:20:30meador.ingesetfiles: + issue-1172711.patch

messages: + msg144003
2011-09-14 00:56:43meador.ingesetmessages: + msg144001
2011-09-13 19:03:19mark.dickinsonsetmessages: + msg143983
2011-09-13 17:52:24meador.ingesetmessages: + msg143980
2011-09-13 11:45:38mark.dickinsonsetmessages: + msg143955
2011-09-13 11:43:27mark.dickinsonsetversions: + Python 3.3, - Python 3.2
2011-09-13 02:15:39meador.ingesetmessages: + msg143948
2011-09-12 19:55:03skrahsetmessages: + msg143934
2011-09-11 08:57:52skrahsetnosy: + skrah
messages: + msg143856
2011-09-09 03:04:19meador.ingesetmessages: + msg143751
2011-09-09 03:02:12meador.ingesetfiles: + issue-1172711.patch
2011-09-08 23:28:33hayposetnosy: + haypo
messages: + msg143745
2011-09-05 10:35:26arigosetnosy: - arigo
2011-09-05 02:25:05meador.ingesetfiles: + issue-1172711.patch

messages: + msg143507
2011-01-12 21:31:47mattchaputsetnosy: + mattchaput
messages: + msg126142
2010-08-24 13:09:48meador.ingesetfiles: + issue-1172711.patch

messages: + msg114784
stage: test needed -> patch review
2010-08-03 14:02:08meador.ingesetnosy: + meador.inge
2010-06-24 12:44:46orsenthilsetversions: + Python 3.2, - Python 2.7
2010-06-24 12:25:36mark.dickinsonsetmessages: + msg108509
2010-06-24 12:21:07mark.dickinsonsetnosy: + mark.dickinson
messages: + msg108508
2010-06-24 04:46:51cmcqueen1975setnosy: + cmcqueen1975
messages: + msg108500
2009-07-03 14:17:46ocean-citysetfiles: - array_long_long.patch
2009-07-03 14:17:37ocean-citysetfiles: + array_long_long.patch
2009-07-03 14:07:17ocean-citysetfiles: + array_long_long.patch
nosy: + ocean-city
messages: + msg90059

2009-02-15 23:33:02ajaksu2setstage: test needed
type: enhancement
components: + Extension Modules, - Library (Lib)
versions: + Python 2.7, - Python 2.5
2005-03-29 18:58:42orenticreate