davidmogar / cucco

Text normalization library for Python
MIT License
203 stars 27 forks source link

Python 2.7 patch #8

Closed xecgr closed 7 years ago

xecgr commented 8 years ago

Hi I needed your lib in a python 2.7 environment. I saw an issue (https://github.com/davidmogar/normalizr/issues/4) that was closed because your lib was implemented to be used with python 3. After some tests, the only change you should do is relative imports inside your lib. I've coded it, and it works like a charm.

Thanks for your lib, is good and simple.

davidmogar commented 8 years ago

Hi @xecgr,

Thank you very much for your pull request and sorry for take so long to review it.

Although your solution works, I think that is better to use try/except for this. So the init file would look like this:

try:
  from normalizr.normalizr import Normalizr
except ImportError:
  from normalizr import Normalizr

and the normalizr.py file:

try:
  import normalizr.regex as regex
except ImportError:
  import regex as regex

If you could change this and squash the commits (current and new) I'll be happy to merge your pull request. I can do it myself if not. Just let me know ;)

Thanks again for your PR, David

benfei commented 8 years ago

Hi @xecgr @davidmogar,

I may suggest to use the following import statement instead of the try-catch clauses. from __future__ import absolute_import

What's your opinion about it?

Cheers, Ben.

davidmogar commented 7 years ago

Hi @feinsteinben,

Yeah, that's another option indeed. Users will still have to append a 'u' in front of texts to normalize (don't apply to all normalizations) but it's not worthy to take a look to that.

Cheers, David

davidmogar commented 7 years ago

Hi @xecgr,

I changed it myself following @feinsteinben suggestion.

Thanks for your input, David

xecgr commented 7 years ago

This solution don't solve the problem. I've attached a prompt output

(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ ls normalizr*
normalizr:
data  __init__.py  __init__.pyc  normalizr.py  normalizr.pyc  regex.py  regex.pyc

normalizr-0.1.9-py2.7.egg-info:
dependency_links.txt  installed-files.txt  PKG-INFO  SOURCES.txt  top_level.txt
(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ rm -rf normalizr*
(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ pip install git+ssh://git@github.com/davidmogar/normalizr.git
Collecting git+ssh://git@github.com/davidmogar/normalizr.git
  Cloning ssh://git@github.com/davidmogar/normalizr.git to /tmp/pip-X7kv50-build
Installing collected packages: normalizr
  Running setup.py install for normalizr
Successfully installed normalizr-0.1.9
You are using pip version 7.1.2, however version 8.1.2 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ python
Python 2.7.6 (default, Jun 22 2015, 17:58:13) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from normalizr import Normalizr
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "normalizr/__init__.py", line 5, in <module>
    from normalizr.normalizr import Normalizr
  File "normalizr/normalizr.py", line 9, in <module>
    import regex
ImportError: No module named regex
>>> 
(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ head normalizr/normalizr.py
from __future__ import absolute_import

import codecs
import logging
import os
import re
import string
import unicodedata
import regex
import sys
(zip_code)xecgr@carteras:~/dev/zip_code/local/lib/python2.7/site-packages$ 

Please reopen the pull request, and we'll discuss which solution is better

davidmogar commented 7 years ago

Fixed now. I pushed a version I shouldn't push...