Skip to content

CLN: remove fill_tuple kludge #33310

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.

Sign up for GitHub

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 2 commits into from
Apr 6, 2020
Merged

CLN: remove fill_tuple kludge #33310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6af41b
CLN: remove fill_tuple kludge
jbrockmendel Apr 6, 2020
4c25d31
dont pass no_defualt
jbrockmendel Apr 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions pandas/core/internals/blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ def func(x):
blocks = [self.make_block_same_class(interp_values)]
return self._maybe_downcast(blocks, downcast)

def take_nd(self, indexer, axis: int, new_mgr_locs=None, fill_tuple=None):
def take_nd(self, indexer, axis: int, new_mgr_locs=None, fill_value=lib.no_default):
"""
Take values according to indexer and return them as a block.bb

Expand All @@ -1252,11 +1252,10 @@ def take_nd(self, indexer, axis: int, new_mgr_locs=None, fill_tuple=None):

values = self.values

if fill_tuple is None:
if fill_value is lib.no_default:
fill_value = self.fill_value
allow_fill = False
else:
fill_value = fill_tuple[0]
allow_fill = True

new_values = algos.take_nd(
Expand Down Expand Up @@ -1721,14 +1720,14 @@ def to_native_types(self, na_rep="nan", quoting=None, **kwargs):
# we are expected to return a 2-d ndarray
return values.reshape(1, len(values))

def take_nd(self, indexer, axis: int = 0, new_mgr_locs=None, fill_tuple=None):
def take_nd(
self, indexer, axis: int = 0, new_mgr_locs=None, fill_value=lib.no_default
):
"""
Take values according to indexer and return them as a block.
"""
if fill_tuple is None:
if fill_value is lib.no_default:
fill_value = None
else:
fill_value = fill_tuple[0]

# axis doesn't matter; we are really a single-dim object
# but are passed the axis depending on the calling routing
Expand Down
23 changes: 9 additions & 14 deletions pandas/core/internals/managers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1299,14 +1299,14 @@ def reindex_indexer(
raise IndexError("Requested axis not found in manager")

if axis == 0:
new_blocks = self._slice_take_blocks_ax0(indexer, fill_tuple=(fill_value,))
new_blocks = self._slice_take_blocks_ax0(indexer, fill_value=fill_value)
else:
new_blocks = [
blk.take_nd(
indexer,
axis=axis,
fill_tuple=(
fill_value if fill_value is not None else blk.fill_value,
fill_value=(
fill_value if fill_value is not None else blk.fill_value
),
)
for blk in self.blocks
Expand All @@ -1317,7 +1317,7 @@ def reindex_indexer(

return type(self).from_blocks(new_blocks, new_axes)

def _slice_take_blocks_ax0(self, slice_or_indexer, fill_tuple=None):
def _slice_take_blocks_ax0(self, slice_or_indexer, fill_value=lib.no_default):
"""
Slice/take blocks along axis=0.

Expand All @@ -1327,7 +1327,7 @@ def _slice_take_blocks_ax0(self, slice_or_indexer, fill_tuple=None):
-------
new_blocks : list of Block
"""
allow_fill = fill_tuple is not None
allow_fill = fill_value is not lib.no_default

sl_type, slobj, sllen = _preprocess_slice_or_indexer(
slice_or_indexer, self.shape[0], allow_fill=allow_fill
Expand All @@ -1339,16 +1339,15 @@ def _slice_take_blocks_ax0(self, slice_or_indexer, fill_tuple=None):
if sl_type in ("slice", "mask"):
return [blk.getitem_block(slobj, new_mgr_locs=slice(0, sllen))]
elif not allow_fill or self.ndim == 1:
if allow_fill and fill_tuple[0] is None:
if allow_fill and fill_value is None:
Copy link
Member

Choose a reason for hiding this comment

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

is allow_fill now redundant here?

Copy link
Contributor

Choose a reason for hiding this comment

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

allow_fill is tricky to remove (but this is a start to it)

Copy link
Member

Choose a reason for hiding this comment

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

The allow_fill is from checking fill_value is not lib.default. So fill_value is None is still a different check I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

what @jorisvandenbossche said

Copy link
Member

Choose a reason for hiding this comment

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

if fill_value is None, isn't allow_fill, by definition, True. so allow_fill is redundant here and if fill_value is None: would be the same? nbd though, i must be missing something. will look again. We no longer need the guard, that was previously required with the fill_tuple __getitem__ call.

_, fill_value = maybe_promote(blk.dtype)
fill_tuple = (fill_value,)

return [
blk.take_nd(
slobj,
axis=0,
new_mgr_locs=slice(0, sllen),
fill_tuple=fill_tuple,
fill_value=fill_value,
)
]

Expand All @@ -1371,8 +1370,7 @@ def _slice_take_blocks_ax0(self, slice_or_indexer, fill_tuple=None):
blocks = []
for blkno, mgr_locs in libinternals.get_blkno_placements(blknos, group=True):
if blkno == -1:
# If we've got here, fill_tuple was not None.
fill_value = fill_tuple[0]
# If we've got here, fill_value was not lib.no_default

blocks.append(
self._make_na_block(placement=mgr_locs, fill_value=fill_value)
Expand All @@ -1393,10 +1391,7 @@ def _slice_take_blocks_ax0(self, slice_or_indexer, fill_tuple=None):
else:
blocks.append(
blk.take_nd(
blklocs[mgr_locs.indexer],
axis=0,
new_mgr_locs=mgr_locs,
fill_tuple=None,
blklocs[mgr_locs.indexer], axis=0, new_mgr_locs=mgr_locs,
)
)

Expand Down