From 89cf9d668aa03099a0546af92c4d5628b2da0c8b Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 12 Aug 2017 10:34:00 -0700 Subject: [PATCH 01/11] Make Period ordinal and freq readonly Remove redundant isinstance(freq, (Tick, DateOffset)) check, since Tick inherits from DateOffset flake8 cleanup of cpython imports --- pandas/_libs/period.pyx | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/period.pyx b/pandas/_libs/period.pyx index 1db31387de5a7..66ee46000f692 100644 --- a/pandas/_libs/period.pyx +++ b/pandas/_libs/period.pyx @@ -3,8 +3,7 @@ import operator from cpython cimport ( PyObject_RichCompareBool, - Py_EQ, Py_NE, -) + Py_EQ, Py_NE) from numpy cimport (int8_t, int32_t, int64_t, import_array, ndarray, NPY_INT64, NPY_DATETIME, NPY_TIMEDELTA) @@ -19,8 +18,11 @@ from pandas import compat from pandas.compat import PY2 cimport cython +# this is _libs.src.datetime, not python stdlib from datetime cimport * + cimport util, lib + from lib cimport is_null_datetimelike, is_period from pandas._libs import tslib, lib from pandas._libs.tslib import (Timedelta, Timestamp, iNaT, @@ -683,13 +685,17 @@ class IncompatibleFrequency(ValueError): cdef class _Period(object): - cdef public: + cdef readonly: int64_t ordinal object freq _comparables = ['name', 'freqstr'] _typ = 'period' + def __cinit__(self, ordinal, freq): + self.ordinal = ordinal + self.freq = freq + @classmethod def _maybe_convert_freq(cls, object freq): @@ -713,9 +719,8 @@ cdef class _Period(object): if ordinal == iNaT: return NaT else: - self = _Period.__new__(cls) - self.ordinal = ordinal - self.freq = cls._maybe_convert_freq(freq) + freq = cls._maybe_convert_freq(freq) + self = _Period.__new__(cls, ordinal, freq) return self def __richcmp__(self, other, op): @@ -767,7 +772,7 @@ cdef class _Period(object): def __add__(self, other): if isinstance(self, Period): if isinstance(other, (timedelta, np.timedelta64, - offsets.Tick, offsets.DateOffset, + offsets.DateOffset, Timedelta)): return self._add_delta(other) elif other is NaT: @@ -785,7 +790,7 @@ cdef class _Period(object): def __sub__(self, other): if isinstance(self, Period): if isinstance(other, (timedelta, np.timedelta64, - offsets.Tick, offsets.DateOffset, + offsets.DateOffset, Timedelta)): neg_other = -other return self + neg_other From e6ca02cb2c036da35a61fc5a6bfc24a62d789366 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 12 Aug 2017 10:35:58 -0700 Subject: [PATCH 02/11] Tests for Period immutability --- pandas/tests/test_lib.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pandas/tests/test_lib.py b/pandas/tests/test_lib.py index 2662720bb436d..fe922234c59db 100644 --- a/pandas/tests/test_lib.py +++ b/pandas/tests/test_lib.py @@ -248,3 +248,14 @@ def test_empty_like(self): expected = np.array([True]) self._check_behavior(arr, expected) + + + +def test_period_immutable(): + per = pd.Period('2014Q1') + with pytest.raises(AttributeError, message="is not writable"): + per.ordinal = 14 + + freq = per.freq + with pytest.raises(AttributeError, message="is not writable"): + per.freq = 2*freq From 891228347688e6b311b28c2159be8ba69f7b62e2 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 12 Aug 2017 20:04:46 -0700 Subject: [PATCH 03/11] Trim whitespace --- pandas/tests/test_lib.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/test_lib.py b/pandas/tests/test_lib.py index fe922234c59db..24f5348a4e8e8 100644 --- a/pandas/tests/test_lib.py +++ b/pandas/tests/test_lib.py @@ -250,7 +250,6 @@ def test_empty_like(self): self._check_behavior(arr, expected) - def test_period_immutable(): per = pd.Period('2014Q1') with pytest.raises(AttributeError, message="is not writable"): From d33922f33fd628c884abd6530965fd45cefd6040 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 12 Aug 2017 20:05:56 -0700 Subject: [PATCH 04/11] flake8 whitespace fixup --- pandas/tests/test_lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/test_lib.py b/pandas/tests/test_lib.py index 24f5348a4e8e8..1ddcc6ac91d20 100644 --- a/pandas/tests/test_lib.py +++ b/pandas/tests/test_lib.py @@ -257,4 +257,4 @@ def test_period_immutable(): freq = per.freq with pytest.raises(AttributeError, message="is not writable"): - per.freq = 2*freq + per.freq = 2 * freq From b86a7b9f455c154bafb674a210afd2b6f0c2915a Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 19 Aug 2017 11:56:31 -0700 Subject: [PATCH 05/11] Add note in whatsnew; address reviewer requests --- doc/source/whatsnew/v0.21.0.txt | 10 ++++++++++ pandas/_libs/period.pyx | 1 - pandas/tests/scalar/test_period.py | 10 ++++++++++ pandas/tests/test_lib.py | 10 ---------- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt index 8b2c4d16f4e1a..29a3d98e890bd 100644 --- a/doc/source/whatsnew/v0.21.0.txt +++ b/doc/source/whatsnew/v0.21.0.txt @@ -132,6 +132,16 @@ Other Enhancements Backwards incompatible API changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +.. _whatsnew_0210.api_breaking.Period: + +pd.Period objects are now immutable +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +:class:`Period` will now raise an ``AttributeError`` when a user tries to +assign a new value to the `ordinal` or `freq` attributes. This will allow +for reliable caching which will improve performance in indexing +operations (:issue:`17116`). + .. _whatsnew_0210.api_breaking.pandas_eval: Improved error handling during item assignment in pd.eval diff --git a/pandas/_libs/period.pyx b/pandas/_libs/period.pyx index 66ee46000f692..d54b834be3009 100644 --- a/pandas/_libs/period.pyx +++ b/pandas/_libs/period.pyx @@ -18,7 +18,6 @@ from pandas import compat from pandas.compat import PY2 cimport cython -# this is _libs.src.datetime, not python stdlib from datetime cimport * cimport util, lib diff --git a/pandas/tests/scalar/test_period.py b/pandas/tests/scalar/test_period.py index 931d6b2b8f1f0..dc8aa348ab536 100644 --- a/pandas/tests/scalar/test_period.py +++ b/pandas/tests/scalar/test_period.py @@ -1406,3 +1406,13 @@ def test_period_ops_offset(self): with tm.assert_raises_regex(period.IncompatibleFrequency, msg): p - offsets.Hour(2) + +def test_period_immutable(): + # see gh-17116 + per = pd.Period('2014Q1') + with pytest.raises(AttributeError, message="is not writable"): + per.ordinal = 14 + + freq = per.freq + with pytest.raises(AttributeError, message="is not writable"): + per.freq = 2 * freq diff --git a/pandas/tests/test_lib.py b/pandas/tests/test_lib.py index 1ddcc6ac91d20..2662720bb436d 100644 --- a/pandas/tests/test_lib.py +++ b/pandas/tests/test_lib.py @@ -248,13 +248,3 @@ def test_empty_like(self): expected = np.array([True]) self._check_behavior(arr, expected) - - -def test_period_immutable(): - per = pd.Period('2014Q1') - with pytest.raises(AttributeError, message="is not writable"): - per.ordinal = 14 - - freq = per.freq - with pytest.raises(AttributeError, message="is not writable"): - per.freq = 2 * freq From 1c6f197208d0122d123a2e61b9cf34798c9ab9ec Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 19 Aug 2017 20:40:06 -0700 Subject: [PATCH 06/11] flake8 whitespace fixup --- pandas/tests/scalar/test_period.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/tests/scalar/test_period.py b/pandas/tests/scalar/test_period.py index dc8aa348ab536..6e5e3b65f854b 100644 --- a/pandas/tests/scalar/test_period.py +++ b/pandas/tests/scalar/test_period.py @@ -1407,6 +1407,7 @@ def test_period_ops_offset(self): with tm.assert_raises_regex(period.IncompatibleFrequency, msg): p - offsets.Hour(2) + def test_period_immutable(): # see gh-17116 per = pd.Period('2014Q1') From 26423668e522167a995d8aa7ca34bb06c22a5928 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sun, 20 Aug 2017 08:34:35 -0700 Subject: [PATCH 07/11] More brevity --- doc/source/whatsnew/v0.21.0.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt index 29a3d98e890bd..023273f28a19f 100644 --- a/doc/source/whatsnew/v0.21.0.txt +++ b/doc/source/whatsnew/v0.21.0.txt @@ -138,9 +138,7 @@ pd.Period objects are now immutable ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ :class:`Period` will now raise an ``AttributeError`` when a user tries to -assign a new value to the `ordinal` or `freq` attributes. This will allow -for reliable caching which will improve performance in indexing -operations (:issue:`17116`). +assign a new value to the `ordinal` or `freq` attributes (:issue:`17116`). .. _whatsnew_0210.api_breaking.pandas_eval: From cd687bf5e384dd6349dbc65aa902f031c64f9a8f Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 23 Aug 2017 11:30:27 -0700 Subject: [PATCH 08/11] Remove message per reviewer suggestion --- pandas/tests/scalar/test_period.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/tests/scalar/test_period.py b/pandas/tests/scalar/test_period.py index 6e5e3b65f854b..a167c9c738b0b 100644 --- a/pandas/tests/scalar/test_period.py +++ b/pandas/tests/scalar/test_period.py @@ -1411,9 +1411,9 @@ def test_period_ops_offset(self): def test_period_immutable(): # see gh-17116 per = pd.Period('2014Q1') - with pytest.raises(AttributeError, message="is not writable"): + with pytest.raises(AttributeError): per.ordinal = 14 freq = per.freq - with pytest.raises(AttributeError, message="is not writable"): + with pytest.raises(AttributeError): per.freq = 2 * freq From dc2111cffecd73a2e9130db432f034cbb4f29d00 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 23 Aug 2017 11:34:48 -0700 Subject: [PATCH 09/11] unwrap line per reviewer request --- doc/source/whatsnew/v0.21.0.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt index ef493741c8710..b876077b871de 100644 --- a/doc/source/whatsnew/v0.21.0.txt +++ b/doc/source/whatsnew/v0.21.0.txt @@ -143,8 +143,7 @@ Backwards incompatible API changes pd.Period objects are now immutable ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -:class:`Period` will now raise an ``AttributeError`` when a user tries to -assign a new value to the `ordinal` or `freq` attributes (:issue:`17116`). +:class:`Period` will now raise an ``AttributeError`` when a user tries to assign a new value to the `ordinal` or `freq` attributes (:issue:`17116`). .. _whatsnew_0210.api_breaking.pandas_eval: From 19567c8f0de212dec15e17dc918e6eada6a8c354 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 23 Aug 2017 16:45:21 -0700 Subject: [PATCH 10/11] Try to guess what jreback wants --- doc/source/whatsnew/v0.21.0.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt index 3062b2001cbe0..d16c264da26cf 100644 --- a/doc/source/whatsnew/v0.21.0.txt +++ b/doc/source/whatsnew/v0.21.0.txt @@ -137,12 +137,6 @@ Other Enhancements Backwards incompatible API changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. _whatsnew_0210.api_breaking.Period: - -pd.Period objects are now immutable -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - :class:`Period` will now raise an ``AttributeError`` when a user tries to assign a new value to the `ordinal` or `freq` attributes (:issue:`17116`). From f739f8f700111f76f8963e4bd02074bfd48f19f8 Mon Sep 17 00:00:00 2001 From: Jeff Reback Date: Thu, 24 Aug 2017 06:02:18 -0400 Subject: [PATCH 11/11] move whatsnew to correct location --- doc/source/whatsnew/v0.21.0.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt index d16c264da26cf..604d275511fa0 100644 --- a/doc/source/whatsnew/v0.21.0.txt +++ b/doc/source/whatsnew/v0.21.0.txt @@ -137,8 +137,6 @@ Other Enhancements Backwards incompatible API changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -:class:`Period` will now raise an ``AttributeError`` when a user tries to assign a new value to the `ordinal` or `freq` attributes (:issue:`17116`). - .. _whatsnew_0210.api_breaking.deps: @@ -293,6 +291,8 @@ Other API Changes - Moved definition of ``MergeError`` to the ``pandas.errors`` module. - The signature of :func:`Series.set_axis` and :func:`DataFrame.set_axis` has been changed from ``set_axis(axis, labels)`` to ``set_axis(labels, axis=0)``, for consistency with the rest of the API. The old signature is deprecated and will show a ``FutureWarning`` (:issue:`14636`) - :func:`Series.argmin` and :func:`Series.argmax` will now raise a ``TypeError`` when used with ``object`` dtypes, instead of a ``ValueError`` (:issue:`13595`) +- :class:`Period` is now immutable, and will now raise an ``AttributeError`` when a user tries to assign a new value to the ``ordinal`` or ``freq`` attributes (:issue:`17116`). + .. _whatsnew_0210.deprecations: