Fix Issue #743: Added exception handling for n_similarity method #882
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix: Word2vec n_similarity returning numpy matrix instead of float with empty list issue #743 in n_similarity() method, if atleast one empty list is passed, it raises ZeroDivisionError.
pranay360
changed the title
tmylk
suggested changes
Sep 25, 2016
| v2 = [self[word] for word in ws2] | ||
| return dot(matutils.unitvec(array(v1).mean(axis=0)), matutils.unitvec(array(v2).mean(axis=0))) | ||
| try: | ||
| assert(len(ws1)>0 and len(ws2)>0) |
| v2 = [self[word] for word in ws2] | ||
| return dot(matutils.unitvec(array(v1).mean(axis=0)), matutils.unitvec(array(v2).mean(axis=0))) | ||
| except AssertionError: | ||
| raise ZeroDivisionError("Atleast one of the passed list is empty.") |
There was a problem hiding this comment.
Please raise ZeroDivisionError directly above. No need for assert.
replace try and catch AssertionError with simple if and else.
Added new test cases in testSimilarities method which makes sure whether ZeroDivisionError is raised if atleast one empty list is passed to word2vec.n_similarities method
piskvorky
reviewed
Sep 26, 2016
| @@ -1,8 +1,9 @@ | |||
| Changes | |||
| ======= | |||
|
|
|||
| 0.13.2, 2016-08-19 | |||
| 0.13.2, 2016-09-25 | |||
There was a problem hiding this comment.
This doesn't look correct -- past release dates cannot change retroactively.
| v1 = [self[word] for word in ws1] | ||
| v2 = [self[word] for word in ws2] | ||
| return dot(matutils.unitvec(array(v1).mean(axis=0)), matutils.unitvec(array(v2).mean(axis=0))) | ||
| if len(ws1) > 0 and len(ws2) > 0: |
There was a problem hiding this comment.
In Python, emptiness of collections is tested simply with if x:
| if len(ws1) > 0 and len(ws2) > 0: | ||
| v1 = [self[word] for word in ws1] | ||
| v2 = [self[word] for word in ws2] | ||
| return dot(matutils.unitvec(array(v1).mean(axis=0)), |
There was a problem hiding this comment.
We don't use vertical indent; please use hanging indent (see PEP8 example).
| matutils.unitvec(array(v2).mean(axis=0))) | ||
| else: | ||
| raise ZeroDivisionError('Atleast one of the passed list is empty.') | ||
| return |
There was a problem hiding this comment.
return makes no sense after a raise...
| @@ -370,9 +370,12 @@ def testSimilarities(self): | |||
| model = word2vec.Word2Vec(size=2, min_count=1, sg=0, hs=0, negative=2) | |||
| model.build_vocab(sentences) | |||
| model.train(sentences) | |||
|
|
|||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix: Word2vec n_similarity returning numpy matrix instead of float with empty list issue #743
in n_similarity() method, if atleast one empty list is passed, it raises ZeroDivisionError.
https://patch-diff.githubusercontent.com/raw/RaRe-Technologies/gensim/pull/882.diff