Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Added new class 'BaseSklearnWrapper' #1383

Merged
merged 10 commits into from
Jun 5, 2017
Merged

Added new class 'BaseSklearnWrapper' #1383

merged 10 commits into from
Jun 5, 2017

Conversation

chinmayapancholi13
Copy link
Contributor

@chinmayapancholi13 chinmayapancholi13 commented Jun 2, 2017 •
edited
Loading

This PR does the following things:

  • Adds new class BaseSklearnWrapper
  • Refactors the code for the sklearn wrappers for LDA and LSI Models by using the BaseSklearnWrapper class to avoid code duplication (due to the function set_params)
  • Fixes the set_params function by using setattr() and also adds unit-tests to avoid this error to go unnoticed in the future.

@menshikh-iv
Copy link
Contributor

Looks good for me 👍

@menshikh-iv menshikh-iv merged commit 89ddb4c into piskvorky:develop Jun 5, 2017
@chinmayapancholi13 chinmayapancholi13 changed the title Added sklearn interface for 'BaseTopicModel' Added new class 'BaseSklearnModel' Jun 6, 2017
@chinmayapancholi13 chinmayapancholi13 changed the title Added new class 'BaseSklearnModel' Added new class 'BaseSklearnWrapper' Jun 6, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chinmayapancholi13 @menshikh-iv