creation of OPENNLP-855 contributed by amensiko

[amensiko]

Sentiment Analysis Parser

[chrismattmann]

thanks @amensiko great initial start. Couple things jump out:

1. remove all DS_Store files
2. we need to change the package names for the OpenNLP stuff to be o.a.opennlp and remove the edu.usc.ir stuff

I can help with this. @kottmann can I get commit access to Apache OpenNLP so I can help out?

[chrismattmann]

Good work removing the DS_Store. The next step would be to take out the edu.usc.ir stuff, and simply add a pom dependency to the SentimentAnalysiParser USC Central repo jar.

[kottmann]

Thanks for the nice work!

I also have a few comments:

• There should be an interface for the Sentiment Analysis Parser e.g. similar to TokenNameFinder or POSTagger. We need this so it is possible to have other implementations of it as well in the future.
• OpenNLP has a command line interface, it would be nice if you could integrate into it.
• SentimentSample should have a toString method, and it should print the sample in the default format (so it can be read in again for training)
• We need evaluation support to know how well it performs (otherwise it can't be improved).
• It would be nice to have tests, especially one which trains and evaluates the performance
• All files in the pull request should have AL headers if possible (e.g. all Java source files)
• In OpenNLP all source files end with a new line

At some point we have to address all of the above, I think it makes sense to get the code quickly into OpenNLP Tools and then continue working on it over there.

I suggest you now do all the changes which can be done quickly and for the ones which take more time you create issues in jira.

[chrismattmann]

Great work @amensiko and great comments @kottmann +1

[chrismattmann]

please remove any commented out code like this

[chrismattmann]

Great work @amensiko I'll take a look here. Can you see my latest comments too.

[amensiko]

Yes @chrismattmann ! Just fixed it.

[chrismattmann]

thanks @amensiko !

[kottmann]

@amensiko could you update this PR for us? It would be nice to get this merged.

[amensiko]

@kottmann I've been having some trouble with it. I'll try to finish it as soon as I figure it out

[kottmann]

Do you have some questions or need some help?

[amensiko]

@kottmann I'm having trouble merging it. Maybe you have some advice?

[kottmann]

Shouldn't be difficult. As far as I can see you only get conflicts in CLI.java and TokenNameFinderTrainerTool.java. The later one shouldn't be changed by you anyway (so this one I would remove from your commit).

I would do it like this:

1. Get the upstream master branch

2. Remove changes to TokenNameFinderTrainerTool.java from your commits or make a new commit to undo the changes you did, e.g.:
   git checkout 4fb05c7 opennlp-tools/src/main/java/opennlp/tools/cmdline/namefind/TokenNameFinderTrainerTool.java
   and then commit that version.

3. Rebase your branch onto master

4. send us a new PR against master (I think you can't update this one)

[kottmann]

Sorry for this, we switched from trunk to master branch, and the deletion of the trunk branch closed this PR. Please send us a new PR based on your branch against the new master branch.