Cleanup / Unicode support #2
Conversation
… functions to their own spot, and made most of the constants a bit more obvious and moved them out so that they don't have to be reinitialized every time. Also, I removed all of the str() calls so that it can now support unicode text
|
Very cool - thanks! Best regards, On Dec 13, 2014, at 4:16 PM, "Kenny Joseph" <[email protected]mailto:[email protected]> wrote: First - thanks so much for posting this code! Its very interesting and has been fun to play with. In starting to use it with my own data, I had two issues that I'm hoping this pull request might aid you and others with. First, it was a little hard to understand what was going on in the sentiment function. In working through it, I separated some stuff out for myself (constants and functions within the function), so I thought it might be useful for others as well. Second, the code couldn't, as far as I could tell, support unicode text, so I made a few mods (basically, I just removed the str() calls and added one conversion at the top of the sentiment function) so that it can now handle unicode. I diff'd the output from this version with the output on the Readme to make sure that all the values were the same, and I'm pretty confident I didn't make any changes that would alter the algorithm. Hopefully they'll be helpful! You can merge this Pull Request by running git pull https://github.com/kennyjoseph/vaderSentiment master Or view, comment on, or merge it at: Commit Summary
File Changes
Patch Links:
Reply to this email directly or view it on GitHubhttps://github.com//pull/2. |
|
Sorry to bring up an old-ish pull request, but is there anything (e.g. unit tests) holding this back from being merged? I ran into the same problem with unicode and came across this PR. It'd be nice to merge this into the master branch if it's okay (and looking at the diff, it seems safe to do so, but I'm admittedly not familiar enough with the code to say). |
|
Thanks for the reminder… it’s merged ☺ Best regards, From: Ali Alkhatib [mailto:[email protected]] Sorry to bring up an old-ish pull request, but is there anything (e.g. unit tests) holding this back from being merged? I ran into the same problem with unicode and came across this PR. It'd be nice to merge this into the master branch if it's okay (and looking at the diff, it seems safe to do so, but I'm admittedly not familiar enough with the code to say). — |
First - thanks so much for posting this code! Its very interesting and has been fun to play with.
In starting to use it with my own data, I had two issues that I'm hoping this pull request might aid you and others with. First, it was a little hard to understand what was going on in the sentiment function. In working through it, I separated some stuff out for myself (constants and functions within the function), so I thought it might be useful for others as well. Second, the code couldn't, as far as I could tell, support unicode text, so I made a few mods (basically, I just removed the str() calls and added one conversion at the top of the sentiment function) so that it can now handle unicode.
I diff'd the output from this version with the output on the Readme to make sure that all the values were the same, and I'm pretty confident I didn't make any changes that would alter the algorithm. Hopefully they'll be helpful!