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

Datetime parsing does recognize am or pm #1289

Merged
merged 7 commits into from
May 4, 2018

Conversation

vkosuri
Copy link
Collaborator

@vkosuri vkosuri commented Apr 29, 2018

@coolrb The script assumes 24hr format by default, to work for you scenario you have to write like this

Scenario 1

from chatterbot import parsing
text= u'Your dental appointment with Dr P. Delvour is scheduled for October 29, 4:00pm. ABC Dentist, 555-555-555.'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)

Update regular expression

re_time = r'(?P<hour>\d{1,2})(\:(?P<minute>\d{1,2})\s?(?P<convention>am|pm))'

Scenario 2:

from chatterbot import parsing
text= u'Your dental appointment with Dr P. Delvour is scheduled for October 29, 16:00. ABC Dentist, 555-555-555.'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)

Scenario 3:
Skip minutes, the regex will work.

from chatterbot import parsing
text= u'Your dental appointment with Dr P. Delvour is scheduled for October 29, 4 pm. ABC Dentist, 555-555-555.'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)

Closes: #1277

@vkosuri vkosuri requested a review from gunthercox April 29, 2018 17:33
@vkosuri
Copy link
Collaborator Author

vkosuri commented May 1, 2018

@gunthercox any comments/suggestions?

@gunthercox
Copy link
Owner

The changes to the regular expression look good to me. I'd just recommend adding two test cases to the unit tests, one for am and one for pm.

@vkosuri
Copy link
Collaborator Author

vkosuri commented May 3, 2018

Fixes #651

Master, will add tests also

from chatterbot import parsing

text = u'Your appointment in next 3 weeks'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)

text = u'Your appointment in next 4 days'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)

text = u'Your appointment in next 10 years'
dates_ch = parsing.datetime_parsing(text)
print(dates_ch)
[('next 3 weeks', datetime.datetime(2018, 5, 24, 0, 0), (20, 32))]
[('next 4 days', datetime.datetime(2018, 5, 7, 0, 0), (20, 31))]
[('next 10 year', datetime.datetime(2023, 11, 21, 0, 0), (20, 32))]

@vkosuri
Copy link
Collaborator Author

vkosuri commented May 3, 2018

I was trying to add more test scenario next 12 months, next day which falls to next month etc.

@vkosuri
Copy link
Collaborator Author

vkosuri commented May 3, 2018

I think this PR addresses this issue also #1291

@vkosuri
Copy link
Collaborator Author

vkosuri commented May 3, 2018

@gunthercox any comments/suggestions?

@gunthercox
Copy link
Owner

@vkosuri This does appear to correct the issue. Very nice work!

@gunthercox gunthercox merged commit e4f6d3a into gunthercox:master May 4, 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.

2 participants