-
-
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
Fix PR #963 : Passing all the params through the apply call in lda.get_document_topics #978
Changes from 9 commits
df53afa
49dce4d
0cc6d3a
102393c
a82fb57
bf94be5
2ac2f95
121a2e2
e01c0a5
387a6ec
587ca88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,6 +249,26 @@ def testGetDocumentTopics(self): | |
self.assertTrue(isinstance(k, int)) | ||
self.assertTrue(isinstance(v, float)) | ||
|
||
#Test case to use the get_document_topic function for the corpus | ||
all_topics = model.get_document_topics(self.corpus, minimum_probability=0.5, minimum_phi_value=0.3, per_word_topics=True) | ||
|
||
self.assertEqual(model.state.numdocs, len(corpus)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the filtering params have no effect? It is a better test if they change the outcome There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am seeing the effect in case I choose minimum_probability=0.8 and minimum_phi_value=1.0, then most of the doc topic list and word phi list will be blank. Should I use that as a test case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep |
||
|
||
for topic in all_topics: | ||
self.assertTrue(isinstance(topic, tuple)) | ||
for k, v in topic[0]: # list of doc_topics | ||
self.assertTrue(isinstance(k, int)) | ||
self.assertTrue(isinstance(v, float)) | ||
|
||
for w, topic_list in topic[1]: # list of word_topics | ||
self.assertTrue(isinstance(w, int)) | ||
self.assertTrue(isinstance(topic_list, list)) | ||
|
||
for w, phi_values in topic[2]: # list of word_phis | ||
self.assertTrue(isinstance(w, int)) | ||
self.assertTrue(isinstance(phi_values, list)) | ||
|
||
|
||
doc_topics, word_topics, word_phis = model.get_document_topics(self.corpus[1], per_word_topics=True) | ||
|
||
for k, v in doc_topics: | ||
|
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 new test for
minimum_prob
andminimum_phi_value
. Select values that change the number of topics and docs returned