Skip to content

Prevent passing invalid kwds to DateOffset constructors #18226

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 35 commits into from
Nov 25, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b9fe60e
Patch __init__ to prevent passing invalid kwds
jbrockmendel Nov 11, 2017
f10a1c6
cast n to integer, assert equality
jbrockmendel Nov 11, 2017
807d769
whatsnew note
jbrockmendel Nov 11, 2017
687e3b7
parameterize tests, define fixture where it is used
jbrockmendel Nov 11, 2017
d5443ca
fixup
jbrockmendel Nov 11, 2017
5e5e0c0
exclude base classes from testing
jbrockmendel Nov 11, 2017
17f7b5a
exclude base classes from testing
jbrockmendel Nov 11, 2017
3834ef8
dummy commit to force CI
jbrockmendel Nov 12, 2017
1c54e96
edits per reviewer suggestions
jbrockmendel Nov 12, 2017
0e37a24
whatsnew whitespace
jbrockmendel Nov 12, 2017
a572368
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 13, 2017
d0ff381
whitespace fixup
jbrockmendel Nov 13, 2017
409dbd0
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 14, 2017
6a9233e
break up hour test to debug appveyor error (segfault?)
jbrockmendel Nov 15, 2017
4406df8
break down segfaulting test to debug
jbrockmendel Nov 16, 2017
44891cd
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 16, 2017
573abb6
fixturize
jbrockmendel Nov 16, 2017
c6cc8bc
troubleshoot segfault by moving __eq__ to _BaseOffset
jbrockmendel Nov 17, 2017
a68f4a7
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 17, 2017
c8224c1
try sorting rd_kwds to fix segfault, revert other troubleshooting gue…
jbrockmendel Nov 17, 2017
55779d8
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 17, 2017
d5b8302
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 19, 2017
b54e26b
implement _validate_n method
jbrockmendel Nov 19, 2017
fade4a2
test for _validate_n
jbrockmendel Nov 19, 2017
38c2238
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 22, 2017
fe895ff
Raise TypeError, not ValueError
jbrockmendel Nov 22, 2017
11ba1a9
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 22, 2017
bc90a19
Catch ValueError in int(n)
jbrockmendel Nov 22, 2017
0b3dca0
fixup extra imports
jbrockmendel Nov 22, 2017
9c841fc
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 22, 2017
adee395
remove unnecessary re-definitions, add tests, improve error msg
jbrockmendel Nov 23, 2017
43dc17e
Add docstring to validate_n
jbrockmendel Nov 23, 2017
4ff8c22
fixup missing format
jbrockmendel Nov 24, 2017
d261be9
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 24, 2017
b4b9e15
Merge branch 'master' of https://github.com/pandas-dev/pandas into ts…
jbrockmendel Nov 25, 2017
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
Next Next commit
Patch __init__ to prevent passing invalid kwds
  • Loading branch information
jbrockmendel committed Nov 13, 2017
commit b9fe60e26156b6884bc5c0c72a96662f4317040c
27 changes: 26 additions & 1 deletion pandas/tests/tseries/test_offsets.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import inspect
import os
from distutils.version import LooseVersion
from datetime import date, datetime, timedelta
Expand All @@ -17,7 +18,7 @@
get_offset, get_standard_freq)
from pandas.core.indexes.datetimes import (
_to_m8, DatetimeIndex, _daterange_cache)
from pandas._libs.tslibs.offsets import WeekDay, CacheableOffset
from pandas._libs.tslibs.offsets import WeekDay, CacheableOffset, _rd_kwds
from pandas.tseries.offsets import (BDay, CDay, BQuarterEnd, BMonthEnd,
BusinessHour, WeekOfMonth, CBMonthEnd,
CustomBusinessHour,
Expand Down Expand Up @@ -4899,3 +4900,27 @@ def test_all_offset_classes(self):
first = Timestamp(test_values[0], tz='US/Eastern') + offset()
second = Timestamp(test_values[1], tz='US/Eastern')
assert first == second


# ---------------------------------------------------------------------

offset_classes = [getattr(offsets, x) for x in dir(offsets)]
Copy link
Contributor

Choose a reason for hiding this comment

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

we already list the classes somewhere in this module iirc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like a fixture defined in conftest, but only used in test_offsets. Instead of checking for the correct types it assume that offsets.__all__ is exactly what it needs. Mind if I a) put it directly test_offsets and b) make it check explicitly to avoid future footgunning?

offset_classes = [x for x in offset_classes if inspect.isclass(x) and
issubclass(x, DateOffset)]


def test_valid_attributes():
Copy link
Contributor

Choose a reason for hiding this comment

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

parameterize this

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

# check that we cannot create e.g. MonthEnd(weeks=3)
month_classes = [x for x in offset_classes if
issubclass(x, offsets.MonthOffset)]

for cls in month_classes:
for kwd in _rd_kwds:
with pytest.raises(TypeError):
cls(**{kwd: 3})

tick_classes = [x for x in offset_classes if issubclass(x, offsets.Tick)]
for cls in tick_classes:
for kwd in _rd_kwds:
with pytest.raises(TypeError):
cls(**{kwd: 4})
23 changes: 23 additions & 0 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,13 @@ def next_bday(self):
class MonthOffset(SingleConstructorOffset):
_adjust_dst = True

def __init__(self, n=1, normalize=False):
self.n = n
self.normalize = normalize
self._offset = timedelta(1)
self._use_relativedelta = False
self.kwds = {}

@property
def name(self):
if self.isAnchored:
Expand Down Expand Up @@ -2471,6 +2478,13 @@ class Easter(DateOffset):
"""
_adjust_dst = True

def __init__(self, n=1, normalize=False):
self.n = n
self.normalize = normalize
Copy link
Contributor

Choose a reason for hiding this comment

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

same (and for other ones you are adding)

self._offset = timedelta(1)
self._use_relativedelta = False
self.kwds = {}

@apply_wraps
def apply(self, other):
currentEaster = easter(other.year)
Expand Down Expand Up @@ -2515,6 +2529,14 @@ class Tick(SingleConstructorOffset):
_inc = Timedelta(microseconds=1000)
_prefix = 'undefined'

def __init__(self, n=1, normalize=False):
# TODO: do Tick classes with normalize=True make sense?
self.n = n
self.normalize = normalize
self._offset = timedelta(1)
self._use_relativedelta = False
self.kwds = {}

__gt__ = _tick_comp(operator.gt)
__ge__ = _tick_comp(operator.ge)
__lt__ = _tick_comp(operator.lt)
Expand Down Expand Up @@ -2573,6 +2595,7 @@ def delta(self):
def nanos(self):
return delta_to_nanoseconds(self.delta)

# TODO: Should Tick have its own apply_index?
def apply(self, other):
# Timestamp can handle tz and nano sec, thus no need to use apply_wraps
if isinstance(other, Timestamp):
Expand Down