classification
Title: Provide PyLong_AsLongAndOverflow compatibility to Python 2.x
Type: enhancement Stage:
Components: Extension Modules Versions: Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: casevh, mark.dickinson
Priority: normal Keywords: patch

Created on 2009-12-17 08:35 by casevh, last changed 2009-12-21 12:42 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
py3intcompat.c casevh, 2009-12-17 08:36
longobject.diff casevh, 2009-12-18 06:52
py3intcompat-v2.c casevh, 2009-12-18 07:29
patch7528.diff casevh, 2009-12-21 05:55 Consolidated patch.
Messages (12)
msg96505 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-17 08:35
When I ported gmpy to Python 3.x, I began to use
PyLong_AsLongAndOverflow frequently. I found the code to slightly faster
and cleaner than using PyLong_AsLong and checking for overflow. I had
several code fragments that looked like:

#if PY_MAJOR_VERSION == 2
    if(PyInt_Check(b)) {
        temp = PyInt_AS_LONG(b));
        Do stuff with temp.
    }
#endif
    if(PyLong_Check(b)) {
#if PY_MAJOR_VERSION == 3
        temp = PyLong_AsLongAndOverflow(b, &overflow);
        if(overflow) {
#else
        temp = PyLong_AsLong(b);
        if(PyErr_Occurred()) {
            PyErr_Clear();
#endif
            Convert b to an mpz.
        } else {
            Do stuff with temp.
        }
    }

I wanted to use the PyLong_AsLongAndOverflow method with Python 2.x so I
extracted the code for PyLong_AsLongAndOverflow, tweeked it to accept
either PyInt or PyLong, and called it PyIntOrLong_AsLongAndOverflow. I
also defined PyIntOrLong_Check.

The same code fragment now looks like:

    if(PyIntOrLong_Check(b)) {
        temp = PyIntOrLong_AsLongAndOverflow(b, &overflow);
        if(overflow) {
            Convert b to an mpz.
        } else {
            Do stuff with temp.
        }
    }

Is it possible to include a py3intcompat.c file with Python 2.7 that
provides this function (and possibly others) for extension authors to
include with their extension? A previous example is pymemcompat.h which
was made available in the Misc directory.

I'm specifically not in favor of adding it to the Python 2.7 API but
just in providing a file for extension authors to use. I've attached a
initial version that compiles successfully with Python 2.4+.

I'm willing to add additional functions, documentation, etc.
msg96506 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-17 08:36
Attached py3intcompat.c
msg96518 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-17 15:08
> I'm specifically not in favor of adding it to the Python 2.7 API.

Out of curiosity, why not?  It seems like a reasonable addition to me.

I'd continue to call it simply PyLong_AsLongAndOverflow, though.  I note
that in 2.x, PyLong_AsLong accepts both ints and longs, so it wouldn't
seem unreasonable for PyLong_AsLongAndOverflow to do so too.
msg96519 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-17 16:42
I don't want it to be a 2.7-only feature. I'm trying to maintain 
compatibility with 2.4 and later for gmpy. I would be in favor with 
adding it to 2.7 as long as we could still provide a standalone version 
for earlier releases of Python.

I hadn't realized that PyLong_AsLong accepts PyInt also. Given that, 
the name PyLong_AsLongAndOverflow is fine (and easier)!
msg96548 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-18 06:52
I uploaded a patch to add PyLong_AsLongAndOverflow. It was made against
the 2.7a1 source. "make test", gmpy's test suite, and mpmath's test
suite all pass.

Let me know if there is anything else I can do.
msg96549 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-18 07:29
Uploaded a new version of py3intcompat.c. It should be save to include
with any version of Python, included 2.7 with PyLong_AsLongAndOverflow
added.

The file includes basic documentation on use.
msg96653 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-19 22:39
Thanks for the patches!

I'll look at the 2.7 PyLong_AsLongAndOverflow patch, and (assuming it 
looks good) apply it.

For the py3intcompat.c, it would be good to have some sort of consensus 
about this;  perhaps it should be discussed on python-dev.  One problem is 
deciding where to put the files---if there might eventually be many such 
files, there should be a reasonable plan.  Those files might also need to 
be divided into helper files for 3-to-2 translation and for 2-to-3 
translation.
msg96669 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-20 10:20
The longobject.diff patch looks fine, modulo some whitespace nits.  (Older 
C source files use width-8 tabs for indentation.)

Are you interested in adding documentation and tests (the latter in the 
test_capi module)?

One thing about the patch struck me as odd:  the use of nb_int means that 
PyLong_AsLongAndOverflow will happily accept floats, Decimal instances, 
etc.  Ideally, this would be nb_index instead, but I guess it has to stay 
nb_int for now, for consistency with PyLong_AsLong.  py3k also uses nb_int 
here and in various other PyLong_As*** functions;  I think these should be 
changed, but that's another issue.
msg96693 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-20 16:33
I will work on documentation and test case patches.

Per comments on python-dev, there doesn't appear to be interest in
distributing forward compatibility files. I will update the bug report
with a slightly revised version of py3intcompat.c and just leave it at that.
msg96731 - (view) Author: Case Van Horsen (casevh) Date: 2009-12-21 05:55
I uploaded a new consolidated diff that includes the original patch with
(hopefully) correct whitespace, some tests, and doc updates.

The test just verifies that overflow is set/cleared properly. Proper
conversions to long are already tested in test_capi. Let me know if I
should add more tests.

I couldn't find any tests for LongAndOverflow in 3.x.
msg96737 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-21 11:26
Perfect---thank you!  Applied to trunk in r76963.  I tweaked the main 
comment and docstring, wrapped a long line, and replaced two instances of 
'*overflow = Py_SIZE(v) > 0 ? 1 : -1' with simply '*overflow = sign'.  I 
also expanded the test a bit to check for 1 and -1 *overflow values, 
rather than just checking for nonzero *overflow.

I'll propagate your tests to py3k.
msg96746 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-21 12:42
I fixed up py3k to be in sync with trunk, and added your tests (slightly 
expanded) to py3k in r76971.  Thanks!
History
Date User Action Args
2009-12-21 12:42:43mark.dickinsonsetstatus: open -> closed
resolution: accepted
messages: + msg96746
2009-12-21 11:26:11mark.dickinsonsetmessages: + msg96737
2009-12-21 05:55:50casevhsetfiles: + patch7528.diff

messages: + msg96731
2009-12-20 16:33:49casevhsetmessages: + msg96693
2009-12-20 10:20:09mark.dickinsonsetmessages: + msg96669
2009-12-19 22:39:16mark.dickinsonsetassignee: mark.dickinson
messages: + msg96653
2009-12-18 07:29:49casevhsetfiles: + py3intcompat-v2.c

messages: + msg96549
2009-12-18 06:52:14casevhsetfiles: + longobject.diff
keywords: + patch
messages: + msg96548
2009-12-17 16:43:00casevhsetmessages: + msg96519
2009-12-17 15:08:05mark.dickinsonsetnosy: + mark.dickinson
messages: + msg96518
2009-12-17 08:36:37casevhsetfiles: + py3intcompat.c

messages: + msg96506
2009-12-17 08:35:36casevhcreate