-
-
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
tests for word2vec's train(). Continuing #1139 #1237
Conversation
|
||
with self.assertRaises(ValueError): | ||
model.train(sentences, epochs=model.iter) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a simple model.train(sentences)
example as well. Thanks
Thanks. Could you please extend this PR by adding the ipynb changes? |
@tmylk Ok, I will resolve the conflicts. Since the some ipynb's codes may take several hours to complete, I am seeking a remote server to run. |
Just updating the code will be fine. There is no need to re-run them. |
…into fix-word2vec-notebook Conflicts: gensim/test/test_word2vec.py
@robotcator Are all the notebooks updated here or is it a work in progress? |
@tmylk all the notebooks have been updated. the commit c9eab32 is the update for notebook, and i also remove my code for fixing the bug of reset_from(). I found this bug when I update the notebook, but it's duplicate in pr1241. |
@robotcator Thanks for finishing this PR |
@@ -474,8 +474,8 @@ def __init__( | |||
if isinstance(sentences, GeneratorType): | |||
raise TypeError("You can't pass a generator as the sentences argument. Try an iterator.") | |||
self.build_vocab(sentences, trim_rule=trim_rule) | |||
self.train(sentences) | |||
|
|||
self.train(sentences, total_examples=self.corpus_count, epochs=self.iter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No vertical indent -- use hanging indent.
def train(self, sentences, total_words=None, word_count=0, | ||
total_examples=None, queue_factor=2, report_delay=1.0): | ||
def train(self, sentences, total_examples=None, total_words=None, | ||
epochs=None, start_alpha=None, end_alpha=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No vertical indent -- use hanging indent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity, what does 'hanging indent' mean in this context? Does the line with epochs=None, …
move right or left? Are lines broken differently?
(FWIW, this "aligned with opening delimiter" style is the first 'yes' example given in PEP8, and used a lot through gensim already.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is function definition, vertical is probably OK here. I misread it as function call when reviewing.
Hanging indent would be arguments on separate lines, with extra indentation (see the PEP8 examples).
add tests to make sure that ValueError is indeed thrown when params are not supplied.
Continuing #1139