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

Force training even if read_only is True #337

Merged
merged 3 commits into from
Oct 12, 2016

Conversation

rmdort
Copy link
Collaborator

@rmdort rmdort commented Oct 10, 2016

This is just a proposal.

Scenario
Most of my clients want a training corpora but they dont want bots to auto-learn.

So when i am starting up the bot

bot = ChatBot('name', read_only=True)
bot.train('english') # Add corpus even if read_only is True

What do you think?

Copy link

@karanluthra karanluthra left a comment

Choose a reason for hiding this comment

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

+1 for the use case. I often had to do run the chatbot twice, once with learning enable to learn from the corpus and then again with learning disabled for the user-facing deployment.

But should this be constrained to exist only for mongo storage adaptor? IMHO it will be useful for users of non mongo storage engines too.

@@ -173,11 +173,11 @@ def filter(self, **kwargs):

return results

def update(self, statement):
def update(self, statement, force = False):
Copy link

@karanluthra karanluthra Oct 10, 2016

Choose a reason for hiding this comment

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

Is this style consistent with rest of the code base?
I have seen foo=bar in method parameters at quite a few places in this code base.
Maybe @gunthercox could take the call.

Copy link
Owner

Choose a reason for hiding this comment

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

I agree with @karanluthra, there should be no spaces when doing assignments in method parameters.

@gunthercox
Copy link
Owner

Tests are currently failing on this pull request so unfortunately I cannot merge it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants