-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fully Python 2 and 3 compatibility #2
base: master
Are you sure you want to change the base?
Conversation
- from future.utils import iteritems, since dict.iteritems() has been deprecated in 3 - NB: this requires that users install future - made print statements compatible - updated some imports ([Qq]ueue and [Cc]onfig[Pp]arser), catching the import error in python 3 that comes from using the old python 2 name - use builtins' input() instead of raw_input()
Otherwise the object's 'self' is undefined and we get a NameError
…of dict.has_key(x); prefix '.' for rel imports also ran code cleanup on utils/nlp.py
Also, just to make it explicit, I license my changes to the code under Apache 2, so you can just integrate it directly into the project. |
|
Thanks for catching that; I guess I was mostly interested in the NN bits of code and missed this one. |
No problem!! Thanks for the compatibility. Always ready to help. |
Does this mean that you are now in charge of maintaining and continuing development on RNNLG? I may have more PRs for you in the future if it's under active development again :) |
Detailed lists of the changes can be found in the commit messages from 9 September 2016.
I've tested this by running the sclstm train and test as described in
README.md
in virtual environments for Python 2.7 and Python 3.5.Note that for Python 3 I needed nltk==3.2.1 instead of 3.0.0.
future
also needs to be installed for compatibility, but otherwise the requirements are basically the same. Instead ofpip install nltk==3.0.0 theano==0.8.2
users need to runpip install nltk theano==0.3.2 future
to get all of the dependencies they need. (Installing always, of course, to user space or a virtual environment.)Requirements list (incl. dependencies installed when installing
theano
):