Hi Chris, On Thu, Jun 29, 2017 at 7:10 PM, Chris Mattmann <mattm...@apache.org> wrote: > Hi Rodrigo, > > This is very useful feedback that I wish we would have had a long time ago. > > I will look into it and see if I can reproduce the CLI error. I did a full > build and mvn > install (which I though would run tests?) before commiting and as I posted in > JIRA > the tests passed for me? So I will have to look into that.
You need to add the tools to the cmdline.CLI, otherwise the tests are not triggered for the sentiment component. https://github.com/apache/opennlp/blob/master/opennlp-tools/src/main/java/opennlp/tools/cmdline/CLI.java > > That said, given your feedback that SentimentME and the Sentiment Component > doesn’t offer much over Document Classifier I agree with you, but wasn’t super > familiar with the Document Classifier API. That said, if we can get the same > functionality > by just using Document Classifier why don’t we: > > 1. Remove the SentimentME and associated code (except for the unit tests) > 2. Use the sample datasets from NetFlix & Stanford Treebank sentiment and > build models using Document Classifier API. > 3. Rename and keep the unit tests that test against Netflix and Stanford tree > bank. > That way we get basic sentiment analysis (that is working for us internally > at JPL decently), > for Apache OpenNLP, and then if we want to build something better than a > Document > Classification approach to sentiment we can do so. > > Thoughts? +1 to implement these three steps. > > Thanks for your useful feedback. If everyone agrees this is a plan I can back > out the code > using Joern’s revert, and then try and execute 1-3 above in a branch first. > Thanks. +1 Cheers, R
