bradfordboyle / pyglpk

Updated fork of T. Finley's PyGLPK module
GNU General Public License v3.0
14 stars 11 forks source link

Python 3 compatibility #3

Closed snorfalorpagus closed 9 years ago

snorfalorpagus commented 9 years ago

This PR brings compatibility with Python 3.x to PyGLPK. It should also support 2.7 (tested) and 2.6 (not tested).

I've taken a least effort approach to the port, following http://python3porting.com/cextensions.html and various other docs. The main changes are in module initialisation in glpk.c and some macros for string->unicode and int->long conversion in py3k.h. I've also run the test modules through 2to3.

Ignoring outstanding issues (https://github.com/bradfordboyle/pyglpk/pull/1 and https://github.com/bradfordboyle/pyglpk/issues/2), only 1 test fails:

TESTS FAILED!! : 1 failures, 1 errors from 261 tests total

The failing test is vectortests.py::testComparisonDifferentCollection - it's not clear to me why this works in Python 2.x, as I can't see an explicit method to compare BarCollection objects.

bradfordboyle commented 9 years ago

I suspect that because a comparison or similar method is not provided, python2 defaults to calling objects.__cmp__ method. See:

https://docs.python.org/2/reference/datamodel.html#object.__cmp__ https://docs.python.org/2/c-api/object.html

My guess would be that a explicitly defined rich comparison methods will have to be provided.

snorfalorpagus commented 9 years ago

You are correct. It hadn't occured to me that object provided a default, and in fact it doesn't in 3.x.

Python 2.7.9 (default, Dec 30 2014, 18:28:09) 
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> o1 = object()
>>> o2 = object()
>>> o1 > o2
False
Python 3.4.2 (default, Nov 23 2014, 01:27:17) 
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.54)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> o1 = object()
>>> o2 = object()
>>> o1 > o2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unorderable types: object() > object()

The == and != comparisons make sense to me, and are needed by Bar (to be equal, the must be in the same BarCollection. But I don't see any logic in making other types of comparison, >, <, etc. Perhaps I am missing something? I suggest implementing EQ and NE explicitly in barcol.c, and raising TypeError for the rest, and adjusting the test to something more sensible. Do you agree?

bradfordboyle commented 9 years ago

The tests seem to be for making sure that the memory allocated for the different collections are non-overlapping and that enough memory has been allocated for them. For the most part, I agree with your assessment and proposed changes. My concern would be potentially breaking old code that depended on the current behavior.

With regards to the overall effort to bring python 3.x compatibility to pyglpk, there are two classes of warnings that pop up when I build on my end. First, the macro PyString_FromFormat in py3k.h is defined multiple times. Second, PySlice_GetIndicesEx() is being passed an incompatible pointer type.

For reference, I am building on Ubuntu 14.04 with python 3.4.0 and gcc 4.8.2.

snorfalorpagus commented 9 years ago

Closing this PR in favor of adding Python 3 support as a pyglpk 0.4.