search

explosion / sense2vec

🦆 Contextually-keyed word vectors
https://explosion.ai/blog/sense2vec-reloaded
MIT License
1.62k stars 240 forks source link

Nonetype error #150

Closed smyja closed 1 year ago

smyja commented 2 years ago

This fixes the nonetype error it throws when the submitted text is problematic.

svlandeg commented 1 year ago

Hi @Smyja, have you had time to look into this question I wrote earlier?

Can you add a unit test that breaks before the PR, and is fixed with it?

The typing of most_similar doesn't actually allow None as a valid value for keys, so I wonder when this comes up. If it happens internally, that might be a bug (and we should raise an error instead of silently continuing). If it's not considered a bug, perhaps we need to expand the typing.

smyja commented 1 year ago

Hi @Smyja, have you had time to look into this question I wrote earlier?

Can you add a unit test that breaks before the PR, and is fixed with it? The typing of most_similar doesn't actually allow None as a valid value for keys, so I wonder when this comes up. If it happens internally, that might be a bug (and we should raise an error instead of silently continuing). If it's not considered a bug, perhaps we need to expand the typing.

For badly formatted texts, you'd get a nonetype error. Can't get the tests folder to run without failing, I'll keep trying it.

svlandeg commented 1 year ago

For badly formatted texts, you'd get a nonetype error.

Can you please give an actual code snippet as example?

rmitsch commented 1 year ago

Closing this due to lack of a response. @Smyja If you are still working on this, feel free to open a new PR - we'd welcome your contribution! :slightly_smiling_face: