-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
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
Changes from 8 commits
4264e15
9f5200a
3eda5b1
fb3d30a
b46d7f9
7a885f3
2d3fe03
783ba6f
bc624ff
88b7340
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -585,17 +571,24 @@ 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_)): | ||
else: | ||
validate_numeric_casting(arr.dtype, value) | ||
|
||
return value | ||
|
||
|
||
cpdef validate_numeric_casting(dtype, object value): | ||
# Note: we can't type dtype as cnp.dtype because that cases dtype.type | ||
# to integer | ||
if (issubclass(dtype.type, (np.integer, np.floating, np.complex)) and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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") | ||
|
||
if issubclass(arr.dtype.type, (np.integer, np.bool_)): | ||
if issubclass(dtype.type, (np.integer, np.bool_)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think much more clear if you reverse these no? (L588 first), then bool casting is out of the way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not clear on what you're suggesting here. There is no bool casting in this function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reverse the order of the if elif, IOW handle the bool dtype.type first |
||
if util.is_float_object(value) and value != value: | ||
raise ValueError("Cannot assign nan to integer series") | ||
|
||
return value | ||
|
||
|
||
cdef class BaseMultiIndexCodesEngine: | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Uh oh!
There was an error while loading. Please reload this page.