Skip to content

algebra cleanup in FY5253 and Easter offsets #18350

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 22, 2017
Merged

algebra cleanup in FY5253 and Easter offsets #18350

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 65 additions & 111 deletions pandas/tseries/offsets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from pandas.core.common import AbstractMethodError

# import after tools, dateutil check
from dateutil.relativedelta import relativedelta, weekday
from dateutil.easter import easter
from pandas._libs import tslib, Timestamp, OutOfBoundsDatetime, Timedelta
from pandas.util._decorators import cache_readonly
Expand Down Expand Up @@ -1938,10 +1937,7 @@ class FY5253(DateOffset):
variation : str
{"nearest", "last"} for "LastOfMonth" or "NearestEndMonth"
"""

_prefix = 'RE'
_suffix_prefix_last = 'L'
_suffix_prefix_nearest = 'N'
_adjust_dst = True

def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1,
Expand All @@ -1963,22 +1959,6 @@ def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1,
raise ValueError('{variation} is not a valid variation'
.format(variation=self.variation))

@cache_readonly
def _relativedelta_forward(self):
if self.variation == "nearest":
weekday_offset = weekday(self.weekday)
return relativedelta(weekday=weekday_offset)
else:
return None

@cache_readonly
def _relativedelta_backward(self):
if self.variation == "nearest":
weekday_offset = weekday(self.weekday)
return relativedelta(weekday=weekday_offset(-1))
else:
return None

@cache_readonly
def _offset_lwom(self):
if self.variation == "nearest":
Expand All @@ -1987,9 +1967,9 @@ def _offset_lwom(self):
return LastWeekOfMonth(n=1, weekday=self.weekday)

def isAnchored(self):
return self.n == 1 \
and self.startingMonth is not None \
and self.weekday is not None
return (self.n == 1 and
self.startingMonth is not None and
self.weekday is not None)

def onOffset(self, dt):
if self.normalize and not _is_normalized(dt):
Expand All @@ -2013,63 +1993,45 @@ def apply(self, other):
datetime(other.year, self.startingMonth, 1))
next_year = self.get_year_end(
datetime(other.year + 1, self.startingMonth, 1))

prev_year = tslib._localize_pydatetime(prev_year, other.tzinfo)
cur_year = tslib._localize_pydatetime(cur_year, other.tzinfo)
next_year = tslib._localize_pydatetime(next_year, other.tzinfo)

if n > 0:
if other == prev_year:
year = other.year - 1
elif other == cur_year:
year = other.year
elif other == next_year:
year = other.year + 1
elif other < prev_year:
year = other.year - 1
n -= 1
if other == prev_year:
n -= 1
elif other == cur_year:
pass
elif other == next_year:
n += 1
# TODO: Not hit in tests
Copy link
Contributor

Choose a reason for hiding this comment

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

can we hit these in tests

Copy link
Member Author

Choose a reason for hiding this comment

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

It wasn't obvious to me how to hit it, so I left the note to try again next time around. (which will be soon. We're far from done with offsets PRs)

Copy link
Member

Choose a reason for hiding this comment

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

@jbrockmendel : If you don't mind explaining, how do you know that this particular branch isn't hit? What about the other branches?

Copy link
Member

Choose a reason for hiding this comment

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

BTW, the changes themselves look OK but would like to get some more color on the questions I just posed to you before merging.

Copy link
Member Author

@jbrockmendel jbrockmendel Nov 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Put in a bunch of assert false statements as a quick coverage check. All the others were hit.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. From which test files did those assert false failures then originate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Almost certainly tests.tseries.offsets.test_offsets

Copy link
Member

@gfyoung gfyoung Nov 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Okay, sounds good. Have a look at those tests once I merge to see how we can increase coverage in this section of logic.

elif n > 0:
if other < prev_year:
n -= 2
# TODO: Not hit in tests
elif other < cur_year:
year = other.year
n -= 1
elif other < next_year:
year = other.year + 1
n -= 1
pass
else:
assert False

result = self.get_year_end(
datetime(year + n, self.startingMonth, 1))

result = datetime(result.year, result.month, result.day,
other.hour, other.minute, other.second,
other.microsecond)
return result
else:
n = -n
if other == prev_year:
year = other.year - 1
elif other == cur_year:
year = other.year
elif other == next_year:
year = other.year + 1
elif other > next_year:
year = other.year + 1
n -= 1
if other > next_year:
n += 2
# TODO: Not hit in tests
elif other > cur_year:
year = other.year
n -= 1
n += 1
elif other > prev_year:
year = other.year - 1
n -= 1
pass
else:
assert False

result = self.get_year_end(
datetime(year - n, self.startingMonth, 1))

result = datetime(result.year, result.month, result.day,
other.hour, other.minute, other.second,
other.microsecond)
return result
shifted = datetime(other.year + n, self.startingMonth, 1)
result = self.get_year_end(shifted)
result = datetime(result.year, result.month, result.day,
other.hour, other.minute, other.second,
other.microsecond)
return result

def get_year_end(self, dt):
if self.variation == "nearest":
Expand All @@ -2078,43 +2040,41 @@ def get_year_end(self, dt):
return self._get_year_end_last(dt)

def get_target_month_end(self, dt):
target_month = datetime(
dt.year, self.startingMonth, 1, tzinfo=dt.tzinfo)
next_month_first_of = shift_month(target_month, 1, None)
return next_month_first_of + timedelta(days=-1)
target_month = datetime(dt.year, self.startingMonth, 1,
tzinfo=dt.tzinfo)
return shift_month(target_month, 0, 'end')
# TODO: is this DST-safe?

def _get_year_end_nearest(self, dt):
target_date = self.get_target_month_end(dt)
if target_date.weekday() == self.weekday:
wkday_diff = self.weekday - target_date.weekday()
if wkday_diff == 0:
return target_date
else:
forward = target_date + self._relativedelta_forward
backward = target_date + self._relativedelta_backward

if forward - target_date < target_date - backward:
return forward
else:
return backward
days_forward = wkday_diff % 7
if days_forward <= 3:
# The upcoming self.weekday is closer than the previous one
return target_date + timedelta(days_forward)
else:
# The previous self.weekday is closer than the upcoming one
return target_date + timedelta(days_forward - 7)

def _get_year_end_last(self, dt):
current_year = datetime(
dt.year, self.startingMonth, 1, tzinfo=dt.tzinfo)
current_year = datetime(dt.year, self.startingMonth, 1,
tzinfo=dt.tzinfo)
return current_year + self._offset_lwom

@property
def rule_code(self):
prefix = self._get_prefix()
prefix = self._prefix
suffix = self.get_rule_code_suffix()
return "{prefix}-{suffix}".format(prefix=prefix, suffix=suffix)

def _get_prefix(self):
return self._prefix

def _get_suffix_prefix(self):
if self.variation == "nearest":
return self._suffix_prefix_nearest
return 'N'
else:
return self._suffix_prefix_last
return 'L'

def get_rule_code_suffix(self):
prefix = self._get_suffix_prefix()
Expand All @@ -2130,17 +2090,15 @@ def _parse_suffix(cls, varion_code, startingMonth_code, weekday_code):
elif varion_code == "L":
variation = "last"
else:
raise ValueError(
"Unable to parse varion_code: {code}".format(code=varion_code))
raise ValueError("Unable to parse varion_code: "
"{code}".format(code=varion_code))

startingMonth = _month_to_int[startingMonth_code]
weekday = _weekday_to_int[weekday_code]

return {
"weekday": weekday,
"startingMonth": startingMonth,
"variation": variation,
}
return {"weekday": weekday,
"startingMonth": startingMonth,
"variation": variation}

@classmethod
def _from_name(cls, *args):
Expand Down Expand Up @@ -2213,10 +2171,9 @@ def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1,

@cache_readonly
def _offset(self):
return FY5253(
startingMonth=self.startingMonth,
weekday=self.weekday,
variation=self.variation)
return FY5253(startingMonth=self.startingMonth,
weekday=self.weekday,
variation=self.variation)

def isAnchored(self):
return self.n == 1 and self._offset.isAnchored()
Expand Down Expand Up @@ -2325,24 +2282,21 @@ class Easter(DateOffset):

@apply_wraps
def apply(self, other):
currentEaster = easter(other.year)
currentEaster = datetime(
currentEaster.year, currentEaster.month, currentEaster.day)
currentEaster = tslib._localize_pydatetime(currentEaster, other.tzinfo)
current_easter = easter(other.year)
current_easter = datetime(current_easter.year,
current_easter.month, current_easter.day)
current_easter = tslib._localize_pydatetime(current_easter,
other.tzinfo)

n = self.n
if n >= 0 and other < current_easter:
n -= 1
elif n < 0 and other > current_easter:
n += 1

# NOTE: easter returns a datetime.date so we have to convert to type of
# other
if self.n >= 0:
if other >= currentEaster:
new = easter(other.year + self.n)
else:
new = easter(other.year + self.n - 1)
else:
if other > currentEaster:
new = easter(other.year + self.n + 1)
else:
new = easter(other.year + self.n)

new = easter(other.year + n)
new = datetime(new.year, new.month, new.day, other.hour,
other.minute, other.second, other.microsecond)
return new
Expand Down