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

LdaMallet show_topic should use topn parameter name to match other topic models #1198

Closed
tmylk opened this issue Mar 8, 2017 · 0 comments
Closed
Labels
difficulty easy Easy issue: required small fix wishlist Feature request

Comments

@tmylk
Copy link
Contributor

tmylk commented Mar 8, 2017

There is an inconsistency between show_topic-topn and show_topics-num_words in LdaModel which is replicated along other topic models in order to standardise the api.

show_topics and print_topics were standardised among topic models in #755 to match LdaModel to use num_words as the argument names were confusing.
Later #771 changed show_topic and print_topic to use num_words just in ldamallet. That was a way to standardise it with show_topics, but in retrospect it was not the best move.
Then print_topic was changed back to use topn like all the other models do when ldamallet inherited from BaseTopicModel. Though show_topic has remained - that is the inconsistency that should be resolved here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty easy Easy issue: required small fix wishlist Feature request
Projects
None yet
Development

No branches or pull requests

1 participant