search

huggingface / neuralcoref

✨Fast Coreference Resolution in spaCy with Neural Networks
https://huggingface.co/coref/
MIT License
2.85k stars 477 forks source link

Update requirements.txt to update spacy #251

Closed vibhavagarwal5 closed 4 years ago

vibhavagarwal5 commented 4 years ago

Removed the restriction to install only for <spacy 2.2.0. The library works well even in the case of spacy 2.2.4. Earlier it used to give an error for ERROR: en-core-web-md 2.2.5 has requirement spacy>=2.2.2, but you'll have spacy 2.1.9 which is incompatible.

Due to this, I propose the following change of updating the spacy version.

vibhavagarwal5 commented 4 years ago

@svlandeg Hi, are you still maintaining this?

svlandeg commented 4 years ago

Thanks for the PR, linking to Issue #197.

What were your steps to get this working with spaCy 2.2.4 ? The main reason we had the upper limit pinned is because newer versions couldn't be installed easily with pip, but required installing from source.

@svlandeg Hi, are you still maintaining this?

See also https://github.com/huggingface/neuralcoref/issues/197#issuecomment-602261288, this status update is still the current status.

vibhavagarwal5 commented 4 years ago

Correct, I had to install from source only, pip wasn't working for me as well. The only reason I updated the spacy version in this because I was getting ERROR: en-core-web-md 2.2.5 has requirement spacy>=2.2.2, but you'll have spacy 2.1.9 which is incompatible. After this the neuralcoref was working perfectly with all the features. Built from source as given in the README file.

git clone https://github.com/huggingface/neuralcoref.git
cd neuralcoref
pip install -r requirements.txt
pip install -e .

By just updating the requirements.txt for spacy 2.2+ it wasn't crashing and no errors.

svlandeg commented 4 years ago

When you got this message: en-core-web-md 2.2.5 has requirement spacy>=2.2.2, but you'll have spacy 2.1.9 which is incompatible. did that actually prevent you from running anything? As far as I know, this would just be a warning from pip, and you would still be able to run everything. I can't imagine it crashing because of the requirements.txt file.

The reason I'm asking is this: if it was just working fine, though with that warning, I'd rather leave the file as is and not merge this PR. Because if we merge, the next user will come and will try to install the latest version of spaCy from pip (not from source), which won't work.

vibhavagarwal5 commented 4 years ago

Yes, I understand your hesitation but even after building from the source the library wasn't working, I was getting Segmentation error: core dumped error whenever I tried doc = nlp("Some text") using the library. After updating my spacy and reinstalling the library with spacy 2.2+, it was working fine. If you have doubts, you can test on yours. I'd be happy to close this PR if that's not the issue on other machines. :smile:

vibhavagarwal5 commented 4 years ago

Hi @svlandeg any plans regarding this PR?