-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Assorted cleanups #27376
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
Assorted cleanups #27376
Changes from 5 commits
f95f4e1
c170f77
16f993d
1cfaf07
dfc3f39
d577ff2
5327c64
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 |
---|---|---|
|
@@ -41,7 +41,6 @@ | |
is_integer_dtype, | ||
is_interval_dtype, | ||
is_list_like, | ||
is_numeric_v_string_like, | ||
is_object_dtype, | ||
is_period_dtype, | ||
is_re, | ||
|
@@ -1297,24 +1296,20 @@ def take_nd(self, indexer, axis, new_mgr_locs=None, fill_tuple=None): | |
|
||
if fill_tuple is None: | ||
fill_value = self.fill_value | ||
new_values = algos.take_nd( | ||
values, indexer, axis=axis, allow_fill=False, fill_value=fill_value | ||
) | ||
allow_fill = False | ||
else: | ||
fill_value = fill_tuple[0] | ||
new_values = algos.take_nd( | ||
values, indexer, axis=axis, allow_fill=True, fill_value=fill_value | ||
) | ||
allow_fill = True | ||
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. In the ExtensionBlock.take_nd method we have this same if/elif for fill_tuple/fill_value, but we don't set allow_fill there. Seems fishy. @jorisvandenbossche any idea if this is intentional? 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. Do you know for what case the In the end, I think this is only an optimization for when you know that there are no -1's in the indexer. Because in the internals, I think we never use the numpy-like indexing semantics of -1 meaning the last element. |
||
|
||
new_values = algos.take_nd( | ||
values, indexer, axis=axis, allow_fill=allow_fill, fill_value=fill_value | ||
) | ||
|
||
# Called from three places in managers, all of which satisfy | ||
# this assertion | ||
assert not (axis == 0 and new_mgr_locs is None) | ||
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 raise a ValueError here instead of using assert? In the off chance someone uses -O flag assert won't help 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. asserts are ok in the internals |
||
if new_mgr_locs is None: | ||
if axis == 0: | ||
slc = libinternals.indexer_as_slice(indexer) | ||
if slc is not None: | ||
new_mgr_locs = self.mgr_locs[slc] | ||
else: | ||
new_mgr_locs = self.mgr_locs[indexer] | ||
else: | ||
new_mgr_locs = self.mgr_locs | ||
new_mgr_locs = self.mgr_locs | ||
|
||
if not is_dtype_equal(new_values.dtype, self.dtype): | ||
return self.make_block(new_values, new_mgr_locs) | ||
|
@@ -1858,11 +1853,11 @@ def take_nd(self, indexer, axis=0, new_mgr_locs=None, fill_tuple=None): | |
# if its REALLY axis 0, then this will be a reindex and not a take | ||
new_values = self.values.take(indexer, fill_value=fill_value, allow_fill=True) | ||
|
||
if self.ndim == 1 and new_mgr_locs is None: | ||
new_mgr_locs = [0] | ||
else: | ||
if new_mgr_locs is None: | ||
new_mgr_locs = self.mgr_locs | ||
# Called from three places in managers, all of which satisfy | ||
# this assertion | ||
assert not (self.ndim == 1 and new_mgr_locs is None) | ||
if new_mgr_locs is None: | ||
new_mgr_locs = self.mgr_locs | ||
|
||
return self.make_block_same_class(new_values, new_mgr_locs) | ||
|
||
|
@@ -3366,10 +3361,6 @@ def _putmask_smart(v, m, n): | |
# if we have nulls | ||
if not _isna_compat(v, nn[0]): | ||
pass | ||
elif is_numeric_v_string_like(nn, v): | ||
# avoid invalid dtype comparisons | ||
# between numbers & strings | ||
pass | ||
elif not (is_float_dtype(nn.dtype) or is_integer_dtype(nn.dtype)): | ||
# only compare integers/floats | ||
pass | ||
|
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 use
Tuple
from typing module and annotate types contained within if possible?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.
will do