danielhers / tupa

Transition-based UCCA Parser
https://danielhers.github.io/tupa
GNU General Public License v3.0
72 stars 24 forks source link

Added missing spaCy requirement to README #4

Closed StefPac closed 7 years ago

StefPac commented 7 years ago

This PR addresses issue #3

danielhers commented 7 years ago

Thanks! setup.py tries to do this automatically though. If it fails, that's another issue.

StefPac commented 7 years ago

I used the pip install tupa branch of the instructions, so I didn't run setup.py.

danielhers commented 7 years ago

Well, pip still runs setup.py behind the scenes, so it should still do it. I'll try to see why it might not work and give a better error message with information on how to download the model manually if it failed. Do you have any logs perhaps?

StefPac commented 7 years ago

These are the spaCy relevant bits:

Collecting spacy>=1.9.0 (from tupa)
  Downloading spacy-1.9.0.tar.gz (3.4MB)
 100% |████████████████████████████████| 3.4MB 294kB/s 
[...]
Running setup.py bdist_wheel for spacy ... 
done
  Stored in directory: /home/spacific/.cache/pip/wheels/8c/a9/96/dba5bf6fdd55d8c04e9ffb1b6244137c36e8b33e03d3e66a9a
danielhers commented 7 years ago

Thanks. If this happens after running setup.py for tupa, then this explains it. I'll try running it myself and see.

danielhers commented 7 years ago

The cause for the problem is that TUPA version 1.1.1 from pip uses a wheel install, which actually doesn't run setup.py and never gets the spaCy model. I think the solution will just be to keep only an sdist (.tar.gz) on PyPI, but currently there's a problem with the sdist for 1.1.1 (it doesn't have requirements.txt) so I need to fix this. Thanks for catching this.