-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix version check in io.py #5
Comments
If I recall correctly, we use more python 2.7 features in our code besides this one. Do you have a particular reason you want to use/support python 2.6? Even if our current code base would work on python2.6 we can't commit to checking each new commit to 2.6, also because that is a python version that simply doesn't exist anymore on our current work stations. |
Oh, Yes I understand. Actually we were to add a new feature into Gensim (Link of PR) which has a dependency on the morfessor package (for loading an already trained morfessor model). And, as we provide support for python 2.6 users as well, as we still have some user base for python 2.6. Also, the fact that I feel, there shouldn't be any more changes/commits apart from this PR for morfessor to work for Python 2.6. So, it would be great if this issue could be resolved. WDYT? |
I merge the patch and made a new release. If you do: pip install Morfessor==2.0.2a4 it should work. If you are providing models to your users, best is to use the 'text' format at this time. We are at this moment reworking the saving of models and the pickles will probably disappear by the next release. |
Thanks a lot @psmit. |
This is in reference to Gensim PR #1067.
On Python 2.6, (Travis Job) the version check in line 18 of morfessor/io.py fails with the following error.
Traceback (most recent call last):
There seems a really simple fix for this issue to get it working for Python 2.6 of using
sys.version_info[0] instead of sys.version_info.major.
If it's fine, I'll go ahead and submit a PR with this fix as we are to integrate that PR into Gensim as well.
The text was updated successfully, but these errors were encountered: