Skip to content

CLN: remove IndexEngine.set_value #31510

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 10 commits into from
Feb 2, 2020
33 changes: 13 additions & 20 deletions pandas/_libs/index.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,6 @@ cdef class IndexEngine:
else:
return get_value_at(arr, loc, tz=tz)

cpdef set_value(self, ndarray arr, object key, object value):
"""
Parameters
----------
arr : 1-dimensional ndarray
"""
cdef:
object loc

loc = self.get_loc(key)
value = convert_scalar(arr, value)

arr[loc] = value

cpdef get_loc(self, object val):
cdef:
Py_ssize_t loc
Expand Down Expand Up @@ -585,16 +571,23 @@ cpdef convert_scalar(ndarray arr, object value):
raise ValueError("cannot set a Timedelta with a non-timedelta "
f"{type(value).__name__}")

if (issubclass(arr.dtype.type, (np.integer, np.floating, np.complex)) and
not issubclass(arr.dtype.type, np.bool_)):
if util.is_bool_object(value):
raise ValueError("Cannot assign bool to float/integer series")
else:
validate_numeric_casting(arr.dtype, value)

return value


if issubclass(arr.dtype.type, (np.integer, np.bool_)):
cpdef validate_numeric_casting(dtype, object value):
# Note: we can't annotate dtype as cnp.dtype because that cases dtype.type
# to integer
if issubclass(dtype.type, (np.integer, np.bool_)):
if util.is_float_object(value) and value != value:
raise ValueError("Cannot assign nan to integer series")

return value
if (issubclass(dtype.type, (np.integer, np.floating, np.complex)) and
Copy link
Contributor

Choose a reason for hiding this comment

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

can you not just do

elsif (.....) as by-definition they are not bools at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

the case where issubclass(dtype.type, np.integer) will pass through in both cases i think

not issubclass(dtype.type, np.bool_)):
if util.is_bool_object(value):
raise ValueError("Cannot assign bool to float/integer series")


cdef class BaseMultiIndexCodesEngine:
Expand Down
10 changes: 7 additions & 3 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

from pandas._config import get_option

from pandas._libs import algos as libalgos, lib, properties
from pandas._libs import algos as libalgos, index as libindex, lib, properties
from pandas._typing import Axes, Axis, Dtype, FilePathOrBuffer, Label, Level, Renamer
from pandas.compat import PY37
from pandas.compat._optional import import_optional_dependency
Expand Down Expand Up @@ -3028,10 +3028,14 @@ def _set_value(self, index, col, value, takeable: bool = False):

series = self._get_item_cache(col)
engine = self.index._engine
engine.set_value(series._values, index, value)
loc = engine.get_loc(index)
libindex.validate_numeric_casting(series.dtype, value)

series._values[loc] = value
# Note: trying to use series._set_value breaks tests in
# tests.frame.indexing.test_indexing and tests.indexing.test_partial
return self
except (KeyError, TypeError):

# set using a non-recursive method & reset the cache
if takeable:
self.iloc[index, col] = value
Expand Down
6 changes: 3 additions & 3 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4650,9 +4650,9 @@ def set_value(self, arr, key, value):
FutureWarning,
stacklevel=2,
)
self._engine.set_value(
com.values_from_object(arr), com.values_from_object(key), value
)
loc = self._engine.get_loc(key)
libindex.validate_numeric_casting(arr.dtype, value)
arr[loc] = value

_index_shared_docs[
"get_indexer_non_unique"
Expand Down
22 changes: 7 additions & 15 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -1026,17 +1026,10 @@ def __setitem__(self, key, value):
self._maybe_update_cacher()

def _set_with_engine(self, key, value):
values = self._values
if is_extension_array_dtype(values.dtype):
# The cython indexing engine does not support ExtensionArrays.
values[self.index.get_loc(key)] = value
return
try:
self.index._engine.set_value(values, key, value)
return
except KeyError:
values[self.index.get_loc(key)] = value
return
# fails with AttributeError for IntervalIndex
loc = self.index._engine.get_loc(key)
libindex.validate_numeric_casting(self.dtype, value)
self._values[loc] = value

def _set_with(self, key, value):
# other: fancy integer or otherwise
Expand Down Expand Up @@ -1116,11 +1109,10 @@ def _set_value(self, label, value, takeable: bool = False):
try:
if takeable:
self._values[label] = value
elif isinstance(self._values, np.ndarray):
# i.e. not EA, so we can use _engine
self.index._engine.set_value(self._values, label, value)
else:
self.loc[label] = value
loc = self.index.get_loc(label)
libindex.validate_numeric_casting(self.dtype, value)
Copy link
Contributor

Choose a reason for hiding this comment

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

would not be averse to you importing validate_numeric_casting at the top

self._values[loc] = value
except KeyError:

# set using a non-recursive method
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/indexing/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def test_setitem_ndarray_3d(self, index, obj, idxr, idxr_id):
r"Buffer has wrong number of dimensions \(expected 1, "
r"got 3\)|"
"'pandas._libs.interval.IntervalTree' object has no attribute "
"'set_value'|" # AttributeError
"'get_loc'|" # AttributeError
"unhashable type: 'numpy.ndarray'|" # TypeError
"No matching signature found|" # TypeError
r"^\[\[\[|" # pandas.core.indexing.IndexingError
Expand Down