Skip to content

Fix invalid relativedelta_kwds #19398

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 9 commits into from
Jul 20, 2018
1 change: 0 additions & 1 deletion doc/source/whatsnew/v0.23.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,6 @@ Offsets
- Bug in :class:`FY5253` where ``datetime`` addition and subtraction incremented incorrectly for dates on the year-end but not normalized to midnight (:issue:`18854`)
- Bug in :class:`FY5253` where date offsets could incorrectly raise an ``AssertionError`` in arithmetic operations (:issue:`14774`)


Numeric
^^^^^^^
- Bug in :class:`Series` constructor with an int or float list where specifying ``dtype=str``, ``dtype='str'`` or ``dtype='U'`` failed to convert the data elements to strings (:issue:`16605`)
Expand Down
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,8 @@ Timezones
Offsets
^^^^^^^

-
- Bug in :class:`FY5253` where date offsets could incorrectly raise an ``AssertionError`` in arithmetic operatons (:issue:`14774`)
- Bug in :class:`DateOffset` where keyword arguments ``week`` and ``milliseconds`` were accepted and ignored. Passing these will now raise ``ValueError`` (:issue:`19398`)
-

Numeric
Expand Down
4 changes: 2 additions & 2 deletions pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,10 @@ def _validate_business_time(t_input):

relativedelta_kwds = set([
'years', 'months', 'weeks', 'days',
'year', 'month', 'week', 'day', 'weekday',
Copy link
Contributor

Choose a reason for hiding this comment

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

technically this is an api change, IOW week & milliseconds cannot be passed anymore (maybe add this PR number somwhere)

Copy link
Member Author

Choose a reason for hiding this comment

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

Shoot you're right.

'year', 'month', 'day', 'weekday',
'hour', 'minute', 'second', 'microsecond',
'nanosecond', 'nanoseconds',
'hours', 'minutes', 'seconds', 'milliseconds', 'microseconds'])
'hours', 'minutes', 'seconds', 'microseconds'])


def _determine_offset(kwds):
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/tseries/offsets/test_offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -3085,6 +3085,13 @@ def test_valid_month_attributes(kwd, month_classes):
cls(**{kwd: 3})


@pytest.mark.parametrize('kwd', sorted(list(liboffsets.relativedelta_kwds)))
def test_valid_relativedelta_kwargs(kwd):
# Check that all the arguments specified in liboffsets.relativedelta_kwds
# are in fact valid relativedelta keyword args
DateOffset(**{kwd: 1})


@pytest.mark.parametrize('kwd', sorted(list(liboffsets.relativedelta_kwds)))
def test_valid_tick_attributes(kwd, tick_classes):
# GH#18226
Expand Down
48 changes: 48 additions & 0 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,54 @@ def __add__(date):
date + BDay(0) == BDay.rollforward(date)

Since 0 is a bit weird, we suggest avoiding its use.

Parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

correct, can you separate this into 2 subsections and make clear the different behavior (via an example)

In [1]: t = pd.Timestamp('20170101 09:10:11')

# operate with the value
In [3]: t + DateOffset(years=2018)
Out[3]: datetime.datetime(4035, 1, 1, 9, 10, 11)

In [5]: t + DateOffset(seconds=1)
Out[5]: Timestamp('2017-01-01 09:10:12')

# set the value
In [6]: t + DateOffset(second=1)
Out[6]: Timestamp('2017-01-01 09:10:01')

In [8]: t + DateOffset(month=1)
Out[8]: Timestamp('2017-01-01 09:10:11')

Copy link
Contributor

Choose a reason for hiding this comment

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

there is an issue to revisit this (IOW we should have 2 different offsets for this different behaviors)

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. The docstring is already pretty big and likely to get much bigger. Is there somewhere else part of it should go?

there is an issue to revisit this (IOW we should have 2 different offsets for this different behaviors)

Yah, IIRC there was a suggestion to have __new__ dispatch some special cases to Tick, MonthOffset, etc. I'm planning to take a swing at that after the immutabilization since __new__ and cython are tricky.

Copy link
Contributor

Choose a reason for hiding this comment

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

don't make it long, make it wide instead (its a bit non-standard ok)

n : int, default 1
normalize : bool, default False

Parameters that ADD to the offset
----------------------------------
years, months, ....

Parameters that REPLACE the offset value
-----------
year, month

----------
n : int, default 1
The number of time periods the offset represents.
normalize : bool, default False
Whether to round the result of a DateOffset addition down to the
previous midnight.
**kwds
Temporal parameter that add to or replace the offset value.

Parameters that **add** to the offset (like Timedelta):

- years
- months
- weeks
- days
- hours
- minutes
- seconds
- microseconds
- nanoseconds

Parameters that **replace** the offset value:

- year
- month
- day
- weekday
- hour
- minute
- second
- microsecond
- nanosecond

See Also
--------
dateutil.relativedelta.relativedelta

Examples
--------
>>> ts = pd.Timestamp('2017-01-01 09:10:11')
>>> ts + DateOffset(months=3)
Timestamp('2017-04-01 09:10:11')

>>> ts = pd.Timestamp('2017-01-01 09:10:11')
>>> ts + DateOffset(month=3)
Timestamp('2017-03-01 09:10:11')
"""
_params = cache_readonly(BaseOffset._params.fget)
_use_relativedelta = False
Expand Down