bear / parsedatetime

Parse human-readable date/time strings
Apache License 2.0
695 stars 106 forks source link

Fix/day of week offsets #153

Closed jonklein closed 8 years ago

jonklein commented 8 years ago

This PR fixes two distinct issues parsing strings with time offsets relative to days of the week.

The first issue is that the modifier "offset" is being turned into a week offset for the day of week. This makes sense for parsing some modifiers ('next', 'last', 'prior', 'previous'), but not for others ('from', 'before', 'after').

The second issue is that even with the previous issue fixed, backwards offsets were not applied properly for day-of-week modifiers. The day of week was resolved properly, but parsing of the remaining chunk was left to the caller and would always offset forward.

The added test cases demonstrate the issues and the fix, as do the examples below.

Before:

>>> datetime.datetime.now()
datetime.datetime(2016, 2, 16, 14, 5, 24, 684154)
>>> cal.parse('Thursday')                 # correct
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=18, tm_hour=14, tm_min=5, tm_sec=34, tm_wday=3, tm_yday=49, tm_isdst=-1), 1)
>>> cal.parse('one day after Thursday')  # should give 2/19
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=26, tm_hour=9, tm_min=0, tm_sec=0, tm_wday=4, tm_yday=57, tm_isdst=-1), 1)
>>> cal.parse('one day before Thursday') # should give 2/17
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=12, tm_hour=9, tm_min=0, tm_sec=0, tm_wday=4, tm_yday=43, tm_isdst=-1), 1)

After:

>>> datetime.datetime.now()
datetime.datetime(2016, 2, 16, 15, 19, 6, 123021)
>>> cal.parse('Thursday')
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=18, tm_hour=15, tm_min=19, tm_sec=10, tm_wday=3, tm_yday=49, tm_isdst=-1), 1)
>>> cal.parse('one day after Thursday')
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=19, tm_hour=9, tm_min=0, tm_sec=0, tm_wday=4, tm_yday=50, tm_isdst=-1), 1)
>>> cal.parse('one day before Thursday')
(time.struct_time(tm_year=2016, tm_mon=2, tm_mday=17, tm_hour=9, tm_min=0, tm_sec=0, tm_wday=2, tm_yday=48, tm_isdst=-1), 1)

Review on Reviewable

codecov-io commented 8 years ago

Current coverage is 77.03%

Merging #153 into master will increase coverage by +0.07% as of 2a987bc

@@            master    #153   diff @@
======================================
  Files           14      14       
  Stmts         1554    1563     +9
  Branches       294     297     +3
  Methods          0       0       
======================================
+ Hit           1196    1204     +8
- Partial         94      95     +1
  Missed         264     264       

Review entire Coverage Diff as of 2a987bc

Powered by Codecov. Updated on successful CI builds.

bear commented 8 years ago

Thanks for the fix and with tests!