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

datafreeze dependency issue #6

Closed
ColdHeat opened this issue Jan 3, 2019 · 6 comments
Closed

datafreeze dependency issue #6

ColdHeat opened this issue Jan 3, 2019 · 6 comments

Comments

@ColdHeat
Copy link

ColdHeat commented Jan 3, 2019

Collecting pyicu>=1.9.3 (from normality>=0.5.1->dataset==1.1.0->-r requirements.txt (line 16))
  Downloading https://files.pythonhosted.org/packages/c2/15/0af20b540c828943b6ffea5677c86e908dcac108813b522adebb75c827c1/PyICU-2.2.tar.gz (211kB)
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-nisBrM/pyicu/setup.py", line 53, in <module>
        ''')
    RuntimeError:
    Please set the ICU_VERSION environment variable to the version of
    ICU you have installed.

Trying to pip install with dataset==1.1.0 is resulting in this error from the python:2.7-alpine Docker image.

ColdHeat added a commit to CTFd/CTFd that referenced this issue Jan 3, 2019
Closes #816
Pin version of normality to work around pudo/normality#6.
@pudo
Copy link
Owner

pudo commented Jan 3, 2019

You can either install py-icu in alpine or pin normality below 1.0.

@pombredanne
Copy link
Contributor

@pudo do you really need the transliteration features of PyICU?
icu is great but this is a native dependency hence the confusion this may bring at times.
I use instead this in ScanCode https://github.com/nexB/scancode-toolkit/blob/9e04eb55cd4a049409b86dac0b76445d87474c3a/src/commoncode/text.py#L151 with @kmike text-unidecode and this is all Python so lighter weight... https://github.com/kmike/text-unidecode

Would you be opened to have the option to use either pyicu or something like what I use as pure python?
ICU is a tad painful to build on Linux, Windows and macOS so a pure Python way would be welcomed.

@pombredanne
Copy link
Contributor

@pudo ping ... this is a problem there aboutcode-org/scancode-toolkit#1463 and not because of normality but because scancode depends on fingerprints. Making icu optional would be much welcomed. I can provide a patch for that either as an optional dep, and/or as a try/except to activate ICU features only if ICU is present.

@pudo
Copy link
Owner

pudo commented Mar 21, 2019

First off, two clarifications: a) this issue should be closed, dataset no longer depends on normality. b) I didn't introduce the hard dependency on pyicu lightly, and I agree that it is a heavyweight dependency that will significantly impact the footprint of an app using this.

The reason that I eventually dropped support for text-unidecode is essentially that it made both normality and fingerprints hard to test and maintain. What normality does is pretty fundamentally a wrapper around transliteration and unicode normalisation. So icu is not tangential, it's the main feature. Using unidecode simply produces different results. This meant that each test assertion in both libraries was brittle. Going further: icu does the thing consistently, unidecode sometimes fails quite badly.

If you wish to commit to maintaining support for both in the long run and throughout both of these libraries, we can talk about it. But I'd really rather recommend you pin to the old version, adopt icu, or even consider forking a separate library that implements unidecode only. Doing both is really going to be ugly throughout.

@ColdHeat ColdHeat changed the title Dataset dependency issue datafreeze dependency issue Mar 22, 2019
@pombredanne
Copy link
Contributor

@pudo thank you for the detailed explanation. As stated in my case this is for fingerprints and I get that you want the highest quality of transliteration. In my case I could deal with much less. Let me see if there is an easy way around thins, possibly with a fake, limited pyicu that I could install before I install fingerprint.
The 10+MB wheel and the pain to build pyicu on all three OSes I support are not worth the pain in my own small use case.

@rgieseke
Copy link

Great thread, just run into install issues with a colleague on a Mac with countrynames which depends on normality ...

@pudo pudo closed this as completed in 1cebd9f Aug 26, 2019
JJwang11 pushed a commit to sigpwny/CTFd-badfork that referenced this issue Jan 22, 2020
Closes CTFd#816
Pin version of normality to work around pudo/normality#6.
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

No branches or pull requests

4 participants