aalto-speech / morfessor

Morfessor is a tool for unsupervised and semi-supervised morphological segmentation
http://morpho.aalto.fi
BSD 2-Clause "Simplified" License
185 stars 29 forks source link

Fix version check in io.py #5

Closed anmolgulati closed 7 years ago

anmolgulati commented 7 years ago

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

  File "/home/travis/build/RaRe-Technologies/gensim/gensim/test/test_varembed_wrapper.py", line 49, in testEnsembleMorphemeEmbeddings
    morfessor_model=varembed_model_morfessor_file, use_morphemes=True)
  File "/home/travis/build/RaRe-Technologies/gensim/gensim/models/wrappers/varembed.py", line 70, in load_varembed_format
    import morfessor
  File "/home/travis/miniconda2/envs/gensim-test/lib/python2.6/site-packages/morfessor/__init__.py", line 29, in <module>
    from .cmd import main, get_default_argparser, main_evaluation, \
  File "/home/travis/miniconda2/envs/gensim-test/lib/python2.6/site-packages/morfessor/cmd.py", line 12, in <module>
    from .io import MorfessorIO
  File "/home/travis/miniconda2/envs/gensim-test/lib/python2.6/site-packages/morfessor/io.py", line 18, in <module>
    PY3 = sys.version_info.major == 3
AttributeError: 'tuple' object has no attribute 'major'

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.

psmit commented 7 years ago

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.

anmolgulati commented 7 years ago

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?

psmit commented 7 years ago

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.

anmolgulati commented 7 years ago

Thanks a lot @psmit. Sounds good. We'll make sure of that. Thanks.