diff --git a/doc/source/whatsnew/v0.19.1.txt b/doc/source/whatsnew/v0.19.1.txt index 147ff8795eb00..150925cfa53eb 100644 --- a/doc/source/whatsnew/v0.19.1.txt +++ b/doc/source/whatsnew/v0.19.1.txt @@ -55,7 +55,9 @@ Bug Fixes - Bug in ``pd.concat`` with dataframes heterogeneous in length and tuple ``keys`` (:issue:`14438`) - Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`) - Bug in ``DataFrame.to_json`` where ``lines=True`` and a value contained a ``}`` character (:issue:`14391`) -- Bug in ``df.groupby`` causing an ``AttributeError`` when grouping a single index frame by a column and the index level (:issue`14327`) +- Bug in ``df.groupby`` causing an ``AttributeError`` when grouping a single +- index frame by a column and the index level (:issue`14327`) - Bug in ``pd.pivot_table`` may raise ``TypeError`` or ``ValueError`` when ``index`` or ``columns`` is not scalar and ``values`` is not specified (:issue:`14380`) +- Bug in ``Series.__setitem__` which allowed mutating read-only arrays (:issue:`14359`). diff --git a/pandas/lib.pyx b/pandas/lib.pyx index ef3407ffd5388..b09a1c2755a06 100644 --- a/pandas/lib.pyx +++ b/pandas/lib.pyx @@ -975,7 +975,9 @@ def astype_intsafe(ndarray[object] arr, new_dtype): if is_datelike and checknull(v): result[i] = NPY_NAT else: - util.set_value_at(result, i, v) + # we can use the unsafe version because we know `result` is mutable + # since it was created from `np.empty` + util.set_value_at_unsafe(result, i, v) return result @@ -986,7 +988,9 @@ cpdef ndarray[object] astype_unicode(ndarray arr): ndarray[object] result = np.empty(n, dtype=object) for i in range(n): - util.set_value_at(result, i, unicode(arr[i])) + # we can use the unsafe version because we know `result` is mutable + # since it was created from `np.empty` + util.set_value_at_unsafe(result, i, unicode(arr[i])) return result @@ -997,7 +1001,9 @@ cpdef ndarray[object] astype_str(ndarray arr): ndarray[object] result = np.empty(n, dtype=object) for i in range(n): - util.set_value_at(result, i, str(arr[i])) + # we can use the unsafe version because we know `result` is mutable + # since it was created from `np.empty` + util.set_value_at_unsafe(result, i, str(arr[i])) return result diff --git a/pandas/src/util.pxd b/pandas/src/util.pxd index fdbfbf62af7d2..be8d0d4aa6302 100644 --- a/pandas/src/util.pxd +++ b/pandas/src/util.pxd @@ -70,7 +70,12 @@ cdef inline object get_value_at(ndarray arr, object loc): return get_value_1d(arr, i) -cdef inline set_value_at(ndarray arr, object loc, object value): +cdef inline set_value_at_unsafe(ndarray arr, object loc, object value): + """Sets a value into the array without checking the writeable flag. + + This should be used when setting values in a loop, check the writeable + flag above the loop and then eschew the check on each iteration. + """ cdef: Py_ssize_t i, sz if is_float_object(loc): @@ -87,6 +92,14 @@ cdef inline set_value_at(ndarray arr, object loc, object value): assign_value_1d(arr, i, value) +cdef inline set_value_at(ndarray arr, object loc, object value): + """Sets a value into the array after checking that the array is mutable. + """ + if not cnp.PyArray_ISWRITEABLE(arr): + raise ValueError('assignment destination is read-only') + + set_value_at_unsafe(arr, loc, value) + cdef inline int is_contiguous(ndarray arr): return cnp.PyArray_CHKFLAGS(arr, cnp.NPY_C_CONTIGUOUS) diff --git a/pandas/tests/series/test_indexing.py b/pandas/tests/series/test_indexing.py index 7c16fd060b181..c44a7a898bb8d 100644 --- a/pandas/tests/series/test_indexing.py +++ b/pandas/tests/series/test_indexing.py @@ -1947,6 +1947,40 @@ def test_multilevel_preserve_name(self): self.assertEqual(result.name, s.name) self.assertEqual(result2.name, s.name) + def test_setitem_scalar_into_readonly_backing_data(self): + # GH14359: test that you cannot mutate a read only buffer + + array = np.zeros(5) + array.flags.writeable = False # make the array immutable + series = Series(array) + + for n in range(len(series)): + with self.assertRaises(ValueError): + series[n] = 1 + + self.assertEqual( + array[n], + 0, + msg='even though the ValueError was raised, the underlying' + ' array was still mutated!', + ) + + def test_setitem_slice_into_readonly_backing_data(self): + # GH14359: test that you cannot mutate a read only buffer + + array = np.zeros(5) + array.flags.writeable = False # make the array immutable + series = Series(array) + + with self.assertRaises(ValueError): + series[1:3] = 1 + + self.assertTrue( + not array.any(), + msg='even though the ValueError was raised, the underlying' + ' array was still mutated!', + ) + if __name__ == '__main__': import nose nose.runmodule(argv=[__file__, '-vvs', '-x', '--pdb', '--pdb-failure'], diff --git a/setup.py b/setup.py index 846e2b7fa2d88..1070ee7aa78c9 100755 --- a/setup.py +++ b/setup.py @@ -476,7 +476,8 @@ def pxd(name): 'pandas/src/period_helper.c']}, index={'pyxfile': 'index', 'sources': ['pandas/src/datetime/np_datetime.c', - 'pandas/src/datetime/np_datetime_strings.c']}, + 'pandas/src/datetime/np_datetime_strings.c'], + 'pxdfiles': ['src/util']}, algos={'pyxfile': 'algos', 'pxdfiles': ['src/util'], 'depends': _pxi_dep['algos']},