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

The parser just recognizes case sensitive strings #1291

Closed
wants to merge 4 commits into from

Conversation

manoelrui
Copy link
Contributor

@gunthercox I tested the parser with some test cases to get datetime objects and I realised that upper letters are crashing the parser when it try to compare strings.

In the first scenario below, as the parser uses the function date_from_relative_week_year, the function argument time assigned with string "Next" is not equals to string "next" and the function will return a "NoneType".

Scenario 1:

from chatterbot import parsing
text = "Next Week"
parser = parsing.datetime_parsing(text)
print(str(parser))

In the second scenario, the parser uses the function convert_time_to_hour_minute. As similar before, the function will return a "NoneType" because the comparison between the convention argument (storing the string "PM") and string "pm" is false.

Scenario 2:

from chatterbot import parsing
text = "2PM"
parser = parsing.datetime_parsing(text)
print(str(parser))

@gunthercox
Copy link
Owner

@manoelrui I think you've correctly identified this bug. Would you be able to add some test cases, similar to what @vkosuri had in his similar pull request?

@manoelrui
Copy link
Contributor Author

@gunthercox I added some test cases to test case-insensitive scenarios. Would you have any suggestions/comments regarding these scenarios or additional scenarios ?

Copy link
Collaborator

@vkosuri vkosuri left a comment

Choose a reason for hiding this comment

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

👍

@gunthercox
Copy link
Owner

@manoelrui The test cases look great, awesome work!

@gunthercox
Copy link
Owner

I've merged the commits 64762ba dbcda9a into master by cherry-picking them. The changes will be available in the next ChatterBot release. @manoelrui Thank you again.

@gunthercox gunthercox closed this May 13, 2018
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