-
-
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
Conversation
pandas/_libs/index.pyx
Outdated
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 comment
The 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 comment
The 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 comment
The 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
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 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
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
ok this is fine |
made possible bc Series._values now returns DTA/TDA for datetime64/timedelta64
Small perf improvement