Skip to content

REF: Make WeekOfMonthMixin a cdef class #34265

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

Merged
merged 7 commits into from
May 25, 2020
2 changes: 2 additions & 0 deletions doc/source/reference/offset_frequency.rst
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ Methods
WeekOfMonth.is_anchored
WeekOfMonth.is_on_offset
WeekOfMonth.__call__
WeekOfMonth.weekday

LastWeekOfMonth
---------------
Expand All @@ -565,6 +566,7 @@ Properties
LastWeekOfMonth.normalize
LastWeekOfMonth.rule_code
LastWeekOfMonth.n
LastWeekOfMonth.weekday

Methods
~~~~~~~
Expand Down
9 changes: 7 additions & 2 deletions pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ cdef class BaseOffset:
exclude = {"n", "inc", "normalize"}
attrs = []
for attr in sorted(self._attributes):
# _attributes instead of __dict__ because cython attrs are not in __dict__
if attr.startswith("_") or attr == "kwds" or not hasattr(self, attr):
# DateOffset may not have some of these attributes
continue
Expand Down Expand Up @@ -1140,13 +1141,17 @@ class CustomMixin:
object.__setattr__(self, "calendar", calendar)


class WeekOfMonthMixin(SingleConstructorOffset):
cdef class WeekOfMonthMixin(SingleConstructorOffset):
"""
Mixin for methods common to WeekOfMonth and LastWeekOfMonth.
"""

cdef readonly:
int weekday

def __init__(self, n=1, normalize=False, weekday=0):
BaseOffset.__init__(self, n, normalize)
object.__setattr__(self, "weekday", weekday)
self.weekday = weekday

if weekday < 0 or weekday > 6:
raise ValueError(f"Day must be 0<=day<=6, got {weekday}")
Expand Down
18 changes: 18 additions & 0 deletions pandas/tests/tseries/offsets/test_offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,19 @@ def test_pickle_v0_15_2(self, datapath):
#
tm.assert_dict_equal(offsets, read_pickle(pickle_path))

def test_pickle_roundtrip(self, offset_types):
off = self._get_offset(offset_types)
res = tm.round_trip_pickle(off)
assert off == res
if type(off) is not DateOffset:
for attr in off._attributes:
if attr == "calendar":
# np.busdaycalendar __eq__ will return False;
# we check holidays and weekmask attrs so are OK
continue
# Make sure nothings got lost from _params (which __eq__) is based on
assert getattr(off, attr) == getattr(res, attr)

def test_onOffset_deprecated(self, offset_types):
# GH#30340 use idiomatic naming
off = self._get_offset(offset_types)
Expand Down Expand Up @@ -3463,6 +3476,11 @@ def test_is_on_offset(self, case):
offset = LastWeekOfMonth(weekday=weekday)
assert offset.is_on_offset(dt) == expected

def test_repr(self):
assert (
repr(LastWeekOfMonth(n=2, weekday=1)) == "<2 * LastWeekOfMonths: weekday=1>"
)


class TestSemiMonthEnd(Base):
_offset = SemiMonthEnd
Expand Down
69 changes: 68 additions & 1 deletion pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,53 @@ def is_on_offset(self, dt):
# TODO, see #1395
return True

def _repr_attrs(self) -> str:
# The DateOffset class differs from other classes in that members
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like it could be integrated to the other classes no? surely a follown

Copy link
Member Author

Choose a reason for hiding this comment

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

yah, there are a lot of __dict__-related workarounds that can be cleaned up once everything is a cdef class

# of self._attributes may not be defined, so we have to use __dict__
# instead.
exclude = {"n", "inc", "normalize"}
attrs = []
for attr in sorted(self.__dict__):
if attr.startswith("_") or attr == "kwds":
continue
elif attr not in exclude:
value = getattr(self, attr)
attrs.append(f"{attr}={value}")

out = ""
if attrs:
out += ": " + ", ".join(attrs)
return out

class BusinessDay(BusinessMixin, SingleConstructorOffset):
@cache_readonly
def _params(self):
"""
Returns a tuple containing all of the attributes needed to evaluate
equality between two DateOffset objects.
"""
# The DateOffset class differs from other classes in that members
# of self._attributes may not be defined, so we have to use __dict__
# instead.
all_paras = self.__dict__.copy()
all_paras["n"] = self.n
all_paras["normalize"] = self.normalize
for key in self.__dict__:
if key not in all_paras:
# cython attributes are not in __dict__
all_paras[key] = getattr(self, key)

if "holidays" in all_paras and not all_paras["holidays"]:
all_paras.pop("holidays")
exclude = ["kwds", "name", "calendar"]
attrs = [
(k, v) for k, v in all_paras.items() if (k not in exclude) and (k[0] != "_")
]
attrs = sorted(set(attrs))
params = tuple([str(type(self))] + attrs)
return params


class BusinessDay(BusinessMixin):
"""
DateOffset subclass representing possibly n business days.
"""
Expand Down Expand Up @@ -785,6 +830,22 @@ def __init__(
BusinessHour.__init__(self, n, normalize, start=start, end=end, offset=offset)
CustomMixin.__init__(self, weekmask, holidays, calendar)

def __reduce__(self):
# None for self.calendar bc np.busdaycalendar doesnt pickle nicely
return (
type(self),
(
self.n,
self.normalize,
self.weekmask,
self.holidays,
None,
self.start,
self.end,
self.offset,
),
)


# ---------------------------------------------------------------------
# Month-Based Offset Classes
Expand Down Expand Up @@ -1311,6 +1372,9 @@ def __init__(self, n=1, normalize=False, week=0, weekday=0):
if self.week < 0 or self.week > 3:
raise ValueError(f"Week must be 0<=week<=3, got {self.week}")

def __reduce__(self):
return type(self), (self.n, self.normalize, self.week, self.weekday)

def _get_offset_day(self, other: datetime) -> int:
"""
Find the day in the same month as other that has the same
Expand Down Expand Up @@ -1370,6 +1434,9 @@ def __init__(self, n=1, normalize=False, weekday=0):
raise ValueError("N cannot be 0")
object.__setattr__(self, "week", -1)

def __reduce__(self):
return type(self), (self.n, self.normalize, self.weekday)

def _get_offset_day(self, other: datetime) -> int:
"""
Find the day in the same month as other that has the same
Expand Down