-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add warning when string is used as argument to infer_vector() #2347
Add warning when string is used as argument to infer_vector() #2347
Conversation
…or docstring clarifications over which I stumbled
This is a welcome sanity-check, thanks! @gojomo any reason the check wasn't previously included, do/did we have a reason to accept strings? |
@tobycheese thanks for PR, please
After, I'll merge your PR |
Only reason for no check previously was that parameter-name/doc-comment were thought clear enough, and perhaps if Python loose typing wants to let a string behave just like a list-of-one-character-strings, why should we complain? (It's not inconcievable that someone would be trying to create a model based on single-char tokens.) But certainly this bites people, and those who need single-char words can just force their strings to be lists, so some sort of warning or error is a good idea. |
Thanks @tobycheese ! A small step for code, one giant leap for user-friendliness :) |
Thanks for commiting, userfriendlyness ftw! |
Another one that I just stumbled over.
infer_vector()
in doc2vec.py requires a list of strings (words). If one does not pay attention and passes a string instead, this does not fail (as strings are lists of characters).infer_vector()
then calculates the vector based on the single characters in the string, which most of the time is not what one wants I think. Imho this merits a warning.