Skip to content

Conversation

@rhalbersma
Copy link

Introduction
These 6 commits hopefully prepare the ground for a full Python 2 / 3 compatible code base.

Rationale
The commits do the following:

  1. I have set up Travis CI and Codecov on my fork in the future branch.
  2. I followed the futurize best practices by running the automated --stage1 and --stage2 steps, keeping all tests green.
  3. futurize does not touch Cython files, so functools.reduce needs to be manually imported.
  4. Running the nosetests with python2 -3 signalled quite a few deprecated warnings concerning user-defined __eq__ functions without an accompanying __hash__ function. I have added hash support consistent with equality: if equality is on a tuple/list of class members, then the hashing will be on the same data structure (tuple or list of the data members of self).
  5. Running python3 setup.py build because of the new Python 3 class system (see this Q&A on StackOverflow).

Status

  • The project builds correctly under python2 and runs all unit tests without error.
  • Running the unit tests with python2 -3 nosetests gives no warnings in code from gambit, and only a handful about system code.
  • Running python3 setup.py build also builds without errors.
  • Running nosetests in Python3 mode fails on all unit tests, mostly because of byte / str incompatibility.

Guidance needed

  • How would you like to tackle Python2 / Python3 string compatibility? My preferred option would be to treat the current cython interface of talking to the C++ code through std::string or char *. This will keep callers using Python2 happy, but requires converting Python3 unicode string literals to byte, e.g. using a .encode('utf-8') member function call.
  • Gambit also produces quite a bit of char * strings from c_str() member functions. When those talk to users, then they need to be marshalled through .decode(). Separating the internal c_str() calls from the ones bubbling up to the interface seems quite a bit of work.
  • There are also bound to be some issues with file() and open() incompatibilities between Python2 and Python3. The way forward is to use open() everywhere, with the appropriate b flags for byte stream reading.

If you agree with the approach taken so far, I can try and plough ahead with the string and file issues. Merging the pull request would eventually be best done in one squashed commit, or piece meal, whatever your preference is.

@rhalbersma
Copy link
Author

@tturocy 4 weeks, no feedback so far, how can I improve this PR?

@tturocy
Copy link
Member

tturocy commented Oct 12, 2018

Many thanks for taking this on. At present I'm on a secondment to a very heavy admin role, which leaves me no time or mindspace for computational game theory. So while I am looking forward to having a closer look at this, I don't quite know just when that might be.

@rhalbersma
Copy link
Author

I have decided to abandon this effort. The remaining obstacles are a bit beyond my Python skills. As a Gambit user, it is considerably less work for me to continue using the existing Gambit Python 2 API and futurize my own user code scripts.

I'll leave this pull request open so that any future attempt to port to Python 3 can use whatever is deemed necessary.

@tturocy
Copy link
Member

tturocy commented Dec 27, 2018

Your efforts on this are appreciated. With the announcement that many of the main scientific python stack packages (like NumPy and pandas) are going to drop Python 2 support for new releases from the start of 2019, this is bubbling up higher in my priority list. However, it's tied up with a number of other packaging issues across the whole project which probably have to be looked at holistically.

@rhalbersma
Copy link
Author

Python 2 itself will no longer be maintained in 12 months time: https://pythonclock.org/

@tturocy
Copy link
Member

tturocy commented Jan 11, 2019

b0f989d now introduces Python 3.7.2 support in the gambit package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants