classification
Title: Remove uses of nb_long slot, and rename to nb_reserved.
Type: behavior Stage:
Components: Versions: Python 3.0, Python 3.1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: benjamin.peterson Nosy List: benjamin.peterson, mark.dickinson
Priority: critical Keywords: patch

Created on 2009-01-10 20:32 by mark.dickinson, last changed 2009-02-11 17:07 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
issue4910_1.patch mark.dickinson, 2009-01-12 09:15 Stage 1 of nb_long removal
issue4910_1a.patch mark.dickinson, 2009-01-12 10:14 Stage 1 nb_long removal (improved patch)
issue4910_2.patch mark.dickinson, 2009-01-15 16:48
issue4910_3.patch mark.dickinson, 2009-01-15 22:26
issue4910_4.patch mark.dickinson, 2009-01-17 22:31 deprecate PyNumber_Int
Messages (13)
msg79571 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-10 20:32
In Python 3.x:

>>> int.__long__.__doc__
'x.__long__() <==> long(x)'

But the long builtin no longer exists.

I'm actually not sure why the nb_long slot still exists in 3.x at all.  
Can anyone enlighten me?
msg79581 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-10 22:16
long(x) replaced by int(x) in r68508.

I'm still wondering about the nb_long slot, though.
msg79655 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-12 09:15
Here's a patch against the py3k branch that gets rid of the two existing 
uses of nb_long in the core:

 - in PyNumber_Long, conversion was attempted first using nb_int and
   then using nb_long.  The patch simply removes the nb_long code, so
   int(x) no longer attempts to use the __long__ method for conversion.

 - in Modules/_struct.c, there's a call to nb_long in a function that's
   attempting to turn an arbitrary PyObject into a PyLongObject;  the
   patch replaces this with nb_int (and updates an error message).

 - In Lib/test/test_long.py, __long__ has been replaced with __int__
   in a test that __int__/__long__ takes precedence over __trunc__
   for conversion to int.

With this patch, all tests pass on my (OS X 10.5/Intel) machine.
(Except test_socket, but I'm 97.2% certain that's unrelated.)

If someone can review this quickly I'll move on to the next patch towards 
removing nb_long.  (My issue #1717 experience suggests that it ought to be 
easier to effect the removal via a series of 3 or 4 short, easy-to-review 
patches with clear intent than via one bigger, more confused patch.)

I think it would be good if nb_long could be altered before 3.0.1.

Benjamin, do you have time to take a look?
msg79659 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-12 10:14
That was a pretty poor patch.  Here's a better one:

 - added Misc/NEWS entry
 - added tests to check that __long__ is never called
 - removed Modules/_struct.c change, in the interests of
   keeping the patch focused.
msg79689 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-12 18:00
The first installment looks good!
msg79902 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-15 16:48
Thanks, Benjamin!  Checked in in r68553, backported to 3.0 in r68556.

Here's the second patch, which fixes almost all remaining uses of nb_long 
but stops short of actually removing/renaming the nb_long slot.

Notes:

(1) I haven't tested the change to PC/winreg.c

(2) The Modules/_struct.c change does introduce a change in behaviour:
for example, before the patch,

struct.pack('q', decimal.Decimal(1))

raises struct.error.  After the patch, the packing succeeds.  I *think*
the patched behaviour is probably the right behaviour, since it agrees 
with 2.x, but it's not 100% clear to me what the intentions of the struct 
module are with respect to integer packing of non-integer types.  This is 
probably a question for another issue.
msg79907 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-15 17:56
On Thu, Jan 15, 2009 at 10:48 AM, Mark Dickinson <report@bugs.python.org> wrote:
>
> Mark Dickinson <dickinsm@gmail.com> added the comment:
>
> Thanks, Benjamin!  Checked in in r68553, backported to 3.0 in r68556.
>
> Here's the second patch, which fixes almost all remaining uses of nb_long
> but stops short of actually removing/renaming the nb_long slot.
>
> Notes:
>
> (1) I haven't tested the change to PC/winreg.c

This looks correct. In fact, I don't really see the point of having
PyHKEY_unaryFailureFunc since a TypeError will automatically be raised
if the slot is NULL, but that is certainly another issue.

>
> (2) The Modules/_struct.c change does introduce a change in behaviour:
> for example, before the patch,
>
> struct.pack('q', decimal.Decimal(1))
>
> raises struct.error.  After the patch, the packing succeeds.  I *think*
> the patched behaviour is probably the right behaviour, since it agrees
> with 2.x, but it's not 100% clear to me what the intentions of the struct
> module are with respect to integer packing of non-integer types.  This is
> probably a question for another issue.

Since Decimal implements __int__ and that's what the struct module is
converting with, I think this is fine.

Overall, the patch looks fine. I wonder if we should mark
PyNumber_Long as deprecated in the docs, though.
msg79913 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-15 19:21
> I wonder if we should mark
> PyNumber_Long as deprecated in the docs, though.

It would be nice to standardize on one of (PyNumber_Long, PyNumber_Int) 
and document the other as deprecated.

Maybe it would make more sense to stick with PyNumber_Long and deprecate 
PyNumber_Int, though?  It would make 2.x -> 3.x transitions easier, and 
would be consistent with some of the rest of the API:  we seem to be 
using PyLong_Check in preference to PyInt_Check in current code...
msg79918 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-15 22:26
Second patch applied in r68619 (py3k) and r68620 (3.0 maintenance branch).

Here's the third and final patch:  rename the nb_long slot to nb_reserved, 
and change its type to (void *).
msg79973 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-16 18:40
+1 for applying the last patch.
msg80031 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-17 18:08
Thanks again for reviewing, Benjamin.  nb_long renamed in r68651, merged 
to 3.0 (along with a few other long->int cleanups) in r68666.

There seems to be a consensus on deprecating PyNumber_Int.  I'll leave 
this open until that's taken care of.
msg80055 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-17 22:31
Here's a patch that deprecates PyNumber_Int in favour of PyNumber_Long:

- move PyNumber_Int definition from abstract.h to intobject.h (and
  fix up comments at the top of intobject.h)
- mark PyNumber_Int as deprecated in the c-api docs, with a promise
  to remove it for 3.1.

I suppose that in theory this goes too far:  we should really deprecate in 
3.1 and remove in 3.2.  But given all the other stuff that's going on for 
3.0.1, this doesn't seem too bad.  Benjamin, what do you think?

N.B.  We should remember to actually remove intobject.h for 3.1. :)
msg81652 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-11 17:07
PyNumber_Int deprecated in r69517, r69518.
History
Date User Action Args
2009-02-11 17:07:06mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg81652
2009-01-17 22:31:38mark.dickinsonsetfiles: + issue4910_4.patch
messages: + msg80055
2009-01-17 18:08:04mark.dickinsonsetmessages: + msg80031
2009-01-16 18:40:42benjamin.petersonsetmessages: + msg79973
2009-01-15 22:26:19mark.dickinsonsetfiles: + issue4910_3.patch
messages: + msg79918
2009-01-15 19:21:28mark.dickinsonsetmessages: + msg79913
2009-01-15 17:57:00benjamin.petersonsetmessages: + msg79907
2009-01-15 16:48:15mark.dickinsonsetfiles: + issue4910_2.patch
messages: + msg79902
2009-01-12 18:00:53benjamin.petersonsetmessages: + msg79689
2009-01-12 10:14:58mark.dickinsonsetfiles: + issue4910_1a.patch
messages: + msg79659
2009-01-12 09:15:28mark.dickinsonsetfiles: + issue4910_1.patch
priority: critical
type: behavior
assignee: benjamin.peterson
title: Should both nb_long and nb_int still exist in 3.x? -> Remove uses of nb_long slot, and rename to nb_reserved.
keywords: + patch
nosy: + benjamin.peterson
messages: + msg79655
2009-01-10 22:16:16mark.dickinsonsetmessages: + msg79581
title: help(int.__long__) refers to nonexistent long function -> Should both nb_long and nb_int still exist in 3.x?
2009-01-10 20:32:22mark.dickinsoncreate