Skip to content

CLN: Remove unused core.internals methods #19250

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 14 commits into from
Jan 21, 2018
Merged
Changes from 11 commits
Commits
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
74 changes: 9 additions & 65 deletions pandas/core/internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,6 @@ def getitem_block(self, slicer, new_mgr_locs=None):
def shape(self):
return self.values.shape

@property
def itemsize(self):
return self.values.itemsize

@property
def dtype(self):
return self.values.dtype
Expand All @@ -327,21 +323,6 @@ def concat_same_type(self, to_concat, placement=None):
return self.make_block_same_class(
values, placement=placement or slice(0, len(values), 1))

def reindex_axis(self, indexer, method=None, axis=1, fill_value=None,
limit=None, mask_info=None):
"""
Reindex using pre-computed indexer information
"""
if axis < 1:
raise AssertionError(
'axis must be at least 1, got {axis}'.format(axis=axis))
if fill_value is None:
fill_value = self.fill_value

new_values = algos.take_nd(self.values, indexer, axis,
fill_value=fill_value, mask_info=mask_info)
return self.make_block(new_values)

def iget(self, i):
return self.values[i]

Expand Down Expand Up @@ -936,11 +917,8 @@ def putmask(self, mask, new, align=True, inplace=False, axis=0,

new_values = self.values if inplace else self.values.copy()

if hasattr(new, 'reindex_axis'):
Copy link
Contributor

Choose a reason for hiding this comment

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

some of these may not be hit, I think I prefer the more idiomatic

new = getattr(new, 'values', new)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I'll check to see whether any value-less cases get through while I'm at it.

new = new.values

if hasattr(mask, 'reindex_axis'):
mask = mask.values
new = getattr(new, 'values', new)
mask = getattr(mask, 'values', mask)

# if we are passed a scalar None, convert it here
if not is_list_like(new) and isna(new) and not self.is_object:
Expand Down Expand Up @@ -1297,8 +1275,7 @@ def eval(self, func, other, errors='raise', try_cast=False, mgr=None):
orig_other = other
values = self.values

if hasattr(other, 'reindex_axis'):
other = other.values
other = getattr(other, 'values', other)

# make sure that we can broadcast
is_transposed = False
Expand Down Expand Up @@ -1446,11 +1423,8 @@ def where(self, other, cond, align=True, errors='raise',
if transpose:
values = values.T

if hasattr(other, 'reindex_axis'):
other = other.values

if hasattr(cond, 'reindex_axis'):
cond = cond.values
other = getattr(other, 'values', other)
cond = getattr(cond, 'values', cond)

# If the default broadcasting would go in the wrong direction, then
# explicitly reshape other instead
Expand Down Expand Up @@ -1926,7 +1900,6 @@ def should_store(self, value):


class DatetimeLikeBlockMixin(object):

@property
def _na_value(self):
return tslib.NaT
Expand Down Expand Up @@ -2614,6 +2587,9 @@ def __init__(self, values, placement, ndim=2, dtype=None):
super(DatetimeTZBlock, self).__init__(values, placement=placement,
ndim=ndim)

# override NonConsolidatableMixIn implementation of get_values
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this for now and revert get_values from the Block.

Copy link
Member Author

Choose a reason for hiding this comment

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

So just keep the unnecessary implementation currently in DatetimeTZBlock?

Copy link
Contributor

Choose a reason for hiding this comment

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

if you want to get this merged. yes. I really don't like this change the way it is. if you want to refactor into something more palable be my guest. but this PR would be on hold until then. so I would simply separate this (and to be honest it would have to be a convincing change).

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do, am just curious because usually you like removing duplicate code.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but in this case you are doing a completely orthognal change, which is already pretty spaghetti like, so a re-factor is in order. and separate that these changes.

get_values = DatetimeLikeBlockMixin.get_values

def copy(self, deep=True, mgr=None):
""" copy constructor """
values = self.values
Expand All @@ -2627,14 +2603,6 @@ def external_values(self):
"""
return self.values.astype('datetime64[ns]').values

def get_values(self, dtype=None):
# return object dtype as Timestamps with the zones
if is_object_dtype(dtype):
f = lambda x: lib.Timestamp(x, tz=self.values.tz)
return lib.map_infer(
self.values.ravel(), f).reshape(self.values.shape)
return self.values

def _slice(self, slicer):
""" return a slice of my values """
if isinstance(slicer, tuple):
Expand Down Expand Up @@ -2760,10 +2728,6 @@ class SparseBlock(NonConsolidatableMixIn, Block):
def shape(self):
return (len(self.mgr_locs), self.sp_index.length)

@property
def itemsize(self):
return self.dtype.itemsize

@property
def fill_value(self):
# return np.nan
Expand Down Expand Up @@ -2887,22 +2851,6 @@ def shift(self, periods, axis=0, mgr=None):
return [self.make_block_same_class(new_values,
placement=self.mgr_locs)]

def reindex_axis(self, indexer, method=None, axis=1, fill_value=None,
limit=None, mask_info=None):
"""
Reindex using pre-computed indexer information
"""
if axis < 1:
raise AssertionError(
'axis must be at least 1, got {axis}'.format(axis=axis))

# taking on the 0th axis always here
if fill_value is None:
fill_value = self.fill_value
return self.make_block_same_class(self.values.take(indexer),
fill_value=fill_value,
placement=self.mgr_locs)

def sparse_reindex(self, new_index):
""" sparse reindex and return a new block
current reindex only works for float64 dtype! """
Expand Down Expand Up @@ -3324,7 +3272,7 @@ def apply(self, f, axes=None, filter=None, do_integrity_check=False,

aligned_args = dict((k, kwargs[k])
for k in align_keys
if hasattr(kwargs[k], 'reindex_axis'))
if hasattr(kwargs[k], 'values'))

for b in self.blocks:
if filter is not None:
Expand Down Expand Up @@ -4552,10 +4500,6 @@ def asobject(self):
"""
return self._block.get_values(dtype=object)

@property
def itemsize(self):
return self._block.values.itemsize

@property
def _can_hold_na(self):
return self._block._can_hold_na
Expand Down