Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dmhowcroft
Copy link

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 of pip install nltk==3.0.0 theano==0.8.2 users need to run pip 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):

future==0.15.2
nltk==3.2.1
numpy==1.11.1
scipy==0.18.0
six==1.10.0
Theano==0.8.2

- 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
@dmhowcroft
Copy link
Author

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.

@ghost
Copy link

ghost commented Jul 2, 2019

da2sents.has_key(key) has to be changed to key in da2sents To run in Latest Python

@dmhowcroft
Copy link
Author

Thanks for catching that; I guess I was mostly interested in the NN bits of code and missed this one.

@ghost
Copy link

ghost commented Jul 2, 2019

No problem!! Thanks for the compatibility. Always ready to help.

@dmhowcroft
Copy link
Author

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 :)

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.

None yet

1 participant