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

Fix/day of week offsets #153

Merged
merged 2 commits into from
Feb 17, 2016
Merged

Fix/day of week offsets #153

merged 2 commits into from
Feb 17, 2016

Conversation

jonklein
Copy link
Collaborator

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
Copy link

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
Copy link
Owner

bear commented Feb 17, 2016

Thanks for the fix and with tests!

bear added a commit that referenced this pull request Feb 17, 2016
@bear bear merged commit e9a3b97 into bear:master Feb 17, 2016
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