-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix negator token lookup on sentiment analysis #27
base: master
Are you sure you want to change the base?
Conversation
Could we get the spec passing on this? Looks like a method is returning a Int32 | Float64 when it should only be returning a Float64 |
It seems to be the same issue as #26, fortunately it was easy to solve! Now it's compiling, but the specs are still failing due to some wrong expecations (maybe related to the |
Hmm there's some weird things happening in the failing specs. Certain numbers are completely off now. |
@watzon it's working again! 🎉 |
Added explicit return type to avoid the following warning:
Related crystal-lang/crystal#8232 |
I just realized this is in the main repo. Sorry I didn't catch it earlier, but PRs should ideally be made to the repo specific to the module that you're modifying. This repo is going to end up just importing all the other repos. So would you be able to make PRs to the proper repos? |
So the specific repo is cadmiumcr/sentiment? I'm having a bad time figuring out where things are located in this small packages setup 😄, e.g., where are stop words located? |
Yep, and stop_words are in i18n :) feel free to come to the gitter if you need help with anything |
The line
item = -item if NEGATORS.includes?(prev_token)
was checking through a Hash, thus it wasn't matching the negator tokens, I changed it to a Set instead.Is this the proper repo or should I send it to https://github.com/cadmiumcr/sentiment?