explosion / spacy-stanza

💥 Use the latest Stanza (StanfordNLP) research models directly in spaCy
MIT License
723 stars 59 forks source link

Relax version requirement for stanfordnlp to include v0.2.0. #15

Closed buhrmann closed 5 years ago

buhrmann commented 5 years ago

After some testing, API-wise the two packages seem to be compatible.

Regarding stanfordnlp outputs, note that in v0.2.0 some POS tag predictions have changed, even in the simple case of the text "Hello world! This is a test." (which is used in the tests). In particular, predictions have flipped in ambiguous cases such "This" and "is" in the second sentence, from DET to PRON and VERB to AUX respectively. I'm not a linguist, but after checking the definitions of the corresponding categories on Wikipedia these predictions seem to be as correct as the previous ones. Hence test are updated to pass in either case.

Feel free to squash the PR!

buhrmann commented 5 years ago

Also, stanfordnlp doesn't seem to be very explicit about how package and model versions fit or don't fit together. To be on the safe side I tested with both at 0.2.0. Perhaps it'd be worth adding a note to the readme?

ines commented 5 years ago

Great, thanks so much!

Also, stanfordnlp doesn't seem to be very explicit about how package and model versions fit or don't fit together.

Ah, that's good to know. (It's probably fine to assume that models downloaded with a version are also compatible with it? I don't know what happens if models are incompatible, but as long as StanfordNLP runs and outputs something, this wrapper should be able to convert it.)