-
Notifications
You must be signed in to change notification settings - Fork 100
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
It should work with new versions of corenlp #639
Comments
Below are tests for TestCoreNLPProcessor. Search for "*** FAILED ***"
|
Below are tests for TestFastNLPProcessor. Search for "*** FAILED ***"
|
Thanks! |
@kwalcock : which branch should I use for these? |
This went with #640. Not sure why I split it up. It is now the modernize branch. |
I pushed some changes to the unit tests, which should make them pass.
For the record, it seems the CoreNLP POS tagger does not follow the UD v2 tags. Neither does the constituent parser. The only that seems to consistently follow them is the dependency tagger that is wrapped in FastNLPProcessor. |
The updated PR is #643 and is under test now. A decision has to be made about the version numbering. Additionally, it might be useful for the Scala code to be able to detect which version of stanford-corenlp it is being "linked" against so that the only difference between the code in a processors v8 and v9 is version.sbt and one line in a build.sbt that defines corenlpV. The tests that have been adjusted would look more like if (stanford.major < 4)
doc.sentences(0).tags.get(3) should be ("TO")
else
doc.sentences(0).tags.get(3) should be ("IN") Then one might not need to go back and forth between branches or commits to see what has changed, keep things in sync, etc. I'm not sure how long that would be sustainable, but it doesn't seem difficult to drop if it doesn't work out. If the stanford library does not supply a version number anywhere, then sbt could supply it via the buildinfo plugin. |
I finally noticed the comment you left
and it made me think even more that this is the way to go. The PR #644 shows how it might work. The change would also be made to the old version 8 branch. |
It is failing some tests in TestCoreNLPProcessor and TestFastNLPProcessor. Details will be pasted below.
The text was updated successfully, but these errors were encountered: