From a9cc9f0bacba87a81ddc461e336c5ee763661707 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Jun 2018 10:07:08 -0700 Subject: [PATCH 1/5] disallow normalize=True with Tick classes --- doc/source/whatsnew/v0.24.0.txt | 2 +- pandas/tests/tseries/offsets/test_offsets.py | 21 ++++++++++++++++++-- pandas/tseries/offsets.py | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index de985d4db5fa3..9311b8a5c4ba4 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -112,7 +112,7 @@ Timezones Offsets ^^^^^^^ -- +- :class:`Tick` subclasses can no longer be created with the argument `normalize=True` (:issue:`????`) - - diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 5369b1a94a956..e0f40b7e74120 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -28,7 +28,7 @@ YearEnd, Day, QuarterEnd, BusinessMonthEnd, FY5253, Nano, Easter, FY5253Quarter, - LastWeekOfMonth) + LastWeekOfMonth, Tick) from pandas.core.tools.datetimes import format, ole2datetime import pandas.tseries.offsets as offsets from pandas.io.pickle import read_pickle @@ -225,6 +225,11 @@ def test_offset_freqstr(self, offset_types): def _check_offsetfunc_works(self, offset, funcname, dt, expected, normalize=False): + + if normalize and issubclass(offset, Tick): + # normalize=True disallowed for Tick subclasses + return + offset_s = self._get_offset(offset, normalize=normalize) func = getattr(offset_s, funcname) @@ -413,6 +418,9 @@ def test_onOffset(self, offset_types): assert offset_s.onOffset(dt) # when normalize=True, onOffset checks time is 00:00:00 + if issubclass(offset_types, Tick): + # normalize=True disallowed for Tick subclasses + return offset_n = self._get_offset(offset_types, normalize=True) assert not offset_n.onOffset(dt) @@ -440,7 +448,9 @@ def test_add(self, offset_types, tz): assert isinstance(result, Timestamp) assert result == expected_localize - # normalize=True + # normalize=True, disallowed for Tick subclasses + if issubclass(offset_types, Tick): + return offset_s = self._get_offset(offset_types, normalize=True) expected = Timestamp(expected.date()) @@ -3140,6 +3150,13 @@ def test_require_integers(offset_types): cls(n=1.5) +def test_tick_normalize_raises(tick_classes): + # check that trying to create a Tick object with normalize=True raises + cls = tick_classes + with pytest.raises(ValueError): + cls(n=3, normalize=True) + + def test_weeks_onoffset(): # GH#18510 Week with weekday = None, normalize = False should always # be onOffset diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index c294110d89ec5..9122064741054 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -2219,6 +2219,9 @@ class Tick(SingleConstructorOffset): def __init__(self, n=1, normalize=False): # TODO: do Tick classes with normalize=True make sense? self.n = self._validate_n(n) + if normalize: + raise ValueError("Tick offset with `normalize=True` are not " + "allowed.") self.normalize = normalize __gt__ = _tick_comp(operator.gt) From b4cbaa91d328c4fb4913d0d6700ec6c4b9508487 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Jun 2018 16:40:49 -0700 Subject: [PATCH 2/5] update with GH references --- doc/source/whatsnew/v0.24.0.txt | 2 +- pandas/tests/tseries/offsets/test_offsets.py | 7 ++++--- pandas/tseries/offsets.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 9311b8a5c4ba4..de75ab53958ba 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -112,7 +112,7 @@ Timezones Offsets ^^^^^^^ -- :class:`Tick` subclasses can no longer be created with the argument `normalize=True` (:issue:`????`) +- :class:`Tick` subclasses can no longer be created with the argument `normalize=True` (:issue:`21427`) - - diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index e0f40b7e74120..c23416638d173 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -227,7 +227,7 @@ def _check_offsetfunc_works(self, offset, funcname, dt, expected, normalize=False): if normalize and issubclass(offset, Tick): - # normalize=True disallowed for Tick subclasses + # normalize=True disallowed for Tick subclasses GH#21427 return offset_s = self._get_offset(offset, normalize=normalize) @@ -419,7 +419,7 @@ def test_onOffset(self, offset_types): # when normalize=True, onOffset checks time is 00:00:00 if issubclass(offset_types, Tick): - # normalize=True disallowed for Tick subclasses + # normalize=True disallowed for Tick subclasses GH#21427 return offset_n = self._get_offset(offset_types, normalize=True) assert not offset_n.onOffset(dt) @@ -448,7 +448,7 @@ def test_add(self, offset_types, tz): assert isinstance(result, Timestamp) assert result == expected_localize - # normalize=True, disallowed for Tick subclasses + # normalize=True, disallowed for Tick subclasses GH#21427 if issubclass(offset_types, Tick): return offset_s = self._get_offset(offset_types, normalize=True) @@ -3152,6 +3152,7 @@ def test_require_integers(offset_types): def test_tick_normalize_raises(tick_classes): # check that trying to create a Tick object with normalize=True raises + # GH#21427 cls = tick_classes with pytest.raises(ValueError): cls(n=3, normalize=True) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 9122064741054..4abeb8123fc46 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -2221,7 +2221,7 @@ def __init__(self, n=1, normalize=False): self.n = self._validate_n(n) if normalize: raise ValueError("Tick offset with `normalize=True` are not " - "allowed.") + "allowed.") # GH#21427 self.normalize = normalize __gt__ = _tick_comp(operator.gt) From 6a28813740404e3f6bde93a4afebff87c114edbb Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Jun 2018 17:49:30 -0700 Subject: [PATCH 3/5] flesh out doc note --- doc/source/whatsnew/v0.24.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index de75ab53958ba..2f69c24bc255c 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -112,7 +112,7 @@ Timezones Offsets ^^^^^^^ -- :class:`Tick` subclasses can no longer be created with the argument `normalize=True` (:issue:`21427`) +- Fixed a bug where :class:``Tick`` subclasses (:class:``Day``, :class:``Hour``, :class:``Minute``, :class:``Second``, :class:``Milli``, :class:``Micro``, :class:``Nano``) could be created with the argument `normalize=True`, which could lead to addition failing to be monotone or associative (:issue:`21427`,:issue:`21434`) - - From 053aa142f9496626acaf310a1bea46de9d89d1bb Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Jun 2018 18:04:27 -0700 Subject: [PATCH 4/5] Whatsnew section --- doc/source/whatsnew/v0.24.0.txt | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 2f69c24bc255c..65cd18d15248a 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -26,6 +26,33 @@ Backwards incompatible API changes .. _whatsnew_0240.api.datetimelike: +Tick DateOffset Normalize Restrictions +-------------------------------------- + +Creating a ``Tick`` object (:class:``Day``, :class:``Hour``, :class:``Minute``, +:class:``Second``, :class:``Milli``, :class:``Micro``, :class:``Nano``) with +`normalize=True` is no longer supported. This prevents unexpected behavior +where addition could fail to be monotone or associative. + +.. ipython:: python + + ts = pd.Timestamp('2018-06-11 18:01:14') + tic = pd.offsets.Hour(n=2, normalize=True) + +Previous Behavior: + +.. code-block:: ipython + + In [2]: ts = pd.Timestamp('2018-06-11 18:01:14') + + In [3]: tic = pd.offsets.Hour(n=2, normalize=True) + + In [4]: ts + tic + Out [4]: Timestamp('2018-06-11 00:00:00') + + In [5]: ts + tic + tic + tic == ts + (tic + tic + tic) + Out [5]: False + Datetimelike API Changes ^^^^^^^^^^^^^^^^^^^^^^^^ From d15d137b7c04acbe83edb32a9b2a61b3dcf7907f Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 13 Jun 2018 08:05:43 -0700 Subject: [PATCH 5/5] requested edits, mostly whatsnew --- doc/source/whatsnew/v0.24.0.txt | 22 +++++++++++++++------- pandas/tseries/offsets.py | 1 - 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 9f5eea8d0874d..43c75cde74b42 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -24,7 +24,7 @@ Other Enhancements Backwards incompatible API changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -.. _whatsnew_0240.api.datetimelike: +.. _whatsnew_0240.api.datetimelike.normalize Tick DateOffset Normalize Restrictions -------------------------------------- @@ -32,27 +32,35 @@ Tick DateOffset Normalize Restrictions Creating a ``Tick`` object (:class:``Day``, :class:``Hour``, :class:``Minute``, :class:``Second``, :class:``Milli``, :class:``Micro``, :class:``Nano``) with `normalize=True` is no longer supported. This prevents unexpected behavior -where addition could fail to be monotone or associative. +where addition could fail to be monotone or associative. (:issue:`21427`) .. ipython:: python ts = pd.Timestamp('2018-06-11 18:01:14') + ts tic = pd.offsets.Hour(n=2, normalize=True) + tic Previous Behavior: .. code-block:: ipython - In [2]: ts = pd.Timestamp('2018-06-11 18:01:14') - - In [3]: tic = pd.offsets.Hour(n=2, normalize=True) - In [4]: ts + tic Out [4]: Timestamp('2018-06-11 00:00:00') In [5]: ts + tic + tic + tic == ts + (tic + tic + tic) Out [5]: False +Current Behavior: + +.. ipython:: python + + tic = pd.offsets.Hour(n=2) + ts + tic + tic + tic == ts + (tic + tic + tic) + + +.. _whatsnew_0240.api.datetimelike: + Datetimelike API Changes ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -139,7 +147,7 @@ Timezones Offsets ^^^^^^^ -- Fixed a bug where :class:``Tick`` subclasses (:class:``Day``, :class:``Hour``, :class:``Minute``, :class:``Second``, :class:``Milli``, :class:``Micro``, :class:``Nano``) could be created with the argument `normalize=True`, which could lead to addition failing to be monotone or associative (:issue:`21427`,:issue:`21434`) +- - - diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 884e09bf14141..ecd15bc7b04b8 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -2217,7 +2217,6 @@ class Tick(SingleConstructorOffset): _attributes = frozenset(['n', 'normalize']) def __init__(self, n=1, normalize=False): - # TODO: do Tick classes with normalize=True make sense? self.n = self._validate_n(n) if normalize: raise ValueError("Tick offset with `normalize=True` are not "