-
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
Make use of django_storage #705
Conversation
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.
This is a fantastic idea. These changes look good, I'm happy to merge them in.
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.
Test are currently failing for code-style checks.
chatterbot/storage/django_storage.py
Outdated
from chatterbot.storage import StorageAdapter | ||
|
||
|
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.
This line should not have been deleted. Python's style guide specifies that there should be two blank lines between imports and the a class definition. Once this is fixed I will merge this pull request.
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.
Thanks Master!. I'll made my changes through PyCharm. I found other issues with Train python manage.py train
with import_module
option isn't working train command. it was failing always. The workaround I have made like this
I tried figured out bug, i couldn't, could you please let me know why it failin and how to solve this problem?
CHATTERBOT_TRAIN = {
'name': 'Test Chatter Bot',
'trainer': 'chatterbot.trainers.ChatterBotCorpusTrainer',
'training_data': [
'./data/greetings.corpus.json'
],
'storage_adapter': 'chatterbot.storage.DjangoStorageAdapter',
'django_app_name': 'howdy',
'use_django_models': True
}
# ChatterBot settings
CHATTERBOT = {
'name': 'Test Chatter Bot',
'logic_adapters': [
{
'import_path': 'chatterbot.logic.BestMatch',
},
{
'import_path': 'chatterbot.logic.LowConfidenceAdapter',
'threshold': 0.75,
'default_response': 'I am sorry, but I do not understand.'
},
{
'import_path': 'chatterbot.logic.SpecificResponseAdapter',
'input_text': 'Help me!',
'output_text': 'Ok, here is a link: http://chatterbot.readthedocs.io/en/stable/'
}
],
'filters': [
'chatterbot.filters.RepetitiveResponseFilter'
],
'storage_adapter': 'chatterbot.storage.DjangoStorageAdapter',
'django_app_name': 'howdy',
'use_django_models': True
}
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.
And also the response and statements are still reversed in django_storage adapter
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.
I'm not sure about the import_module
, feel free to open a new issue for it.
@gunthercox Master, any plans to release chatterbot minor version this week? |
Yes, possibly this weekend. |
@gunthercox Any updates? |
@vkosuri Apologies. I haven't had a chance to get around to drafting a new release yet. There is still a few possible issues that I want to investigate with the new SQL Alchemy adapter before doing the release. |
Currently the django_storage adapter tightly coupled with django_chatterbot. By introduce this fix, others can reuse storage adapter.
@gunthercox please provide your comments/suggestions on this PR.