Skip to content
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 #1230. Fix word2vec reset_from bug in v1.0.1 and added unittest #1234

Merged
merged 34 commits into from
Mar 27, 2017
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
1c63c9a
Merge branch 'release-0.12.3rc1'
tmylk Nov 5, 2015
280a488
Merge branch 'release-0.12.3'
tmylk Nov 6, 2015
ddeb002
Merge branch 'release-0.12.3'
tmylk Nov 6, 2015
f2ac3a9
Update CHANGELOG.txt
tmylk Nov 6, 2015
cf09e8c
Update CHANGELOG.txt
tmylk Nov 6, 2015
b61287a
resolve merge conflict in Changelog
tmylk Jan 29, 2016
3ade404
Merge branch 'release-0.12.4' with #596
tmylk Jan 31, 2016
9e6522e
Merge branch 'release-0.13.0'
tmylk Jun 10, 2016
87c4e9c
Merge branch 'release-0.13.0'
tmylk Jun 10, 2016
9c74b40
Release version typo fix
tmylk Jun 10, 2016
7b30025
Merge branch 'release-0.13.0rc1'
tmylk Jun 10, 2016
de79c8e
Merge branch 'release-0.13.0'
tmylk Jun 22, 2016
d4f9cc5
Merge branch 'release-0.13.1'
tmylk Jun 23, 2016
d8e9c0f
Merge branch 'release-0.13.2'
tmylk Aug 26, 2016
7c118fc
Merge branch 'release-0.13.2'
tmylk Aug 26, 2016
432f840
Merge branch 'release-0.13.3'
tmylk Oct 20, 2016
b42e181
Merge branch 'release-0.13.3'
tmylk Oct 21, 2016
3067cb0
Win and OSX build fix
tmylk Oct 21, 2016
e838391
Merge branch 'release-0.13.4'
tmylk Dec 25, 2016
5d47ec4
Merge branch 'release-0.13.4.1'
tmylk Jan 4, 2017
a18de8d
Merge branch 'release-1.0.0rc1'
tmylk Jan 31, 2017
67b1a17
Typo in version
tmylk Jan 31, 2017
df13670
Fix merge conflict
tmylk Feb 17, 2017
78da89a
Merge branch 'release-1.0.0'
tmylk Feb 24, 2017
fb3f303
Merge branch 'release-1.0.1'
tmylk Mar 3, 2017
adc447d
Merge branch 'release-1.0.1'
tmylk Mar 3, 2017
333fd4d
Merge branch 'release-1.0.1'
tmylk Mar 3, 2017
f3f6c20
Merge branch 'master' of https://github.com/Kreiswolke/gensim into de…
Mar 22, 2017
faf6847
Upgraded to match word2vec class structure
Mar 22, 2017
520d89e
Added unittest for reset_from
Mar 22, 2017
9449d06
Fixed typo
Mar 23, 2017
5c3aca6
Positive reset_from() unittest
Mar 23, 2017
e00a861
Change unittest to check if attributes are shared
Mar 24, 2017
8e82566
Formatting fixed
Mar 24, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions gensim/models/word2vec.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,8 @@ def reset_from(self, other_model):
Borrow shareable pre-built structures (like vocab) from the other_model. Useful
if testing multiple models in parallel on the same corpus.
"""
self.wv.vocab = other_model.vocab
self.wv.index2word = other_model.index2word
self.wv.vocab = other_model.wv.vocab
self.wv.index2word = other_model.wv.index2word
self.cum_table = other_model.cum_table
self.corpus_count = other_model.corpus_count
self.reset_weights()
Expand Down
10 changes: 10 additions & 0 deletions gensim/test/test_word2vec.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,16 @@ def test_sentences_should_not_be_a_generator(self):
def testLoadOnClassError(self):
"""Test if exception is raised when loading word2vec model on instance"""
self.assertRaises(AttributeError, load_on_instance)

def test_reset_from(self):
"""Test if exception is raised when reset_from is used"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there some reason that exception should be raised when reset_from is used? correct behavious is no exception. please add checks that the model is actually reset.

model = word2vec.Word2Vec(sentences, min_count=1)
model2 = word2vec.Word2Vec(new_sentences, min_count=1)
try:
model.reset_from(model2)
except AttributeError:
self.fail('AttributeError in reset_from()')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of this line? what would be different if it was removed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because an uncaught error will result in a test failure, this catch-then-fail seems superfluous - it's enough to try the reset_from(), and do some afterwards checking that it's had the intended effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it is not needed and I just updated the test.


#endclass TestWord2VecModel

class TestWMD(unittest.TestCase):
Expand Down