CLN: separate raising from non-raising parts of method (pandas-dev#27151)

* separate coerce_values from coerce_args

Author: jbrockmendel

pandas/core/internals/blocks.py

@@ -208,17 +208,15 @@ def array_dtype(self):
         """
         return self.dtype
 
-    def make_block(self, values, placement=None, ndim=None):
+    def make_block(self, values, placement=None):
         """
         Create a new block, with type inference propagate any values that are
         not specified
         """
         if placement is None:
             placement = self.mgr_locs
-        if ndim is None:
-            ndim = self.ndim
 
-        return make_block(values, placement=placement, ndim=ndim)
+        return make_block(values, placement=placement, ndim=self.ndim)
 
     def make_block_same_class(self, values, placement=None, ndim=None,
                               dtype=None):
@@ -369,7 +367,9 @@ def fillna(self, value, limit=None, inplace=False, downcast=None):
 
         # fillna, but if we cannot coerce, then try again as an ObjectBlock
         try:
-            values, _ = self._try_coerce_args(self.values, value)
+            # Note: we only call try_coerce_args to let it raise
+            self._try_coerce_args(value)
+
             blocks = self.putmask(mask, value, inplace=inplace)
             blocks = [b.make_block(values=self._try_coerce_result(b.values))
                       for b in blocks]
@@ -659,7 +659,21 @@ def _try_cast_result(self, result, dtype=None):
         # may need to change the dtype here
         return maybe_downcast_to_dtype(result, dtype)
 
-    def _try_coerce_args(self, values, other):
+    def _coerce_values(self, values):
+        """
+        Coerce values (usually derived from self.values) for an operation.
+
+        Parameters
+        ----------
+        values : ndarray or ExtensionArray
+
+        Returns
+        -------
+        ndarray or ExtensionArray
+        """
+        return values
+
+    def _try_coerce_args(self, other):
         """ provide coercion to our input arguments """
 
         if np.any(notna(other)) and not self._can_hold_element(other):
@@ -669,7 +683,7 @@ def _try_coerce_args(self, values, other):
                 type(other).__name__,
                 type(self).__name__.lower().replace('Block', '')))
 
-        return values, other
+        return other
 
     def _try_coerce_result(self, result):
         """ reverse of try_coerce_args """
@@ -718,9 +732,9 @@ def replace(self, to_replace, value, inplace=False, filter=None,
 
         # try to replace, if we raise an error, convert to ObjectBlock and
         # retry
+        values = self._coerce_values(self.values)
         try:
-            values, to_replace = self._try_coerce_args(self.values,
-                                                       to_replace)
+            to_replace = self._try_coerce_args(to_replace)
         except (TypeError, ValueError):
             # GH 22083, TypeError or ValueError occurred within error handling
             # causes infinite loop. Cast and retry only if not objectblock.
@@ -793,7 +807,8 @@ def setitem(self, indexer, value):
         # coerce if block dtype can store value
         values = self.values
         try:
-            values, value = self._try_coerce_args(values, value)
+            value = self._try_coerce_args(value)
+            values = self._coerce_values(values)
             # can keep its own dtype
             if hasattr(value, 'dtype') and is_dtype_equal(values.dtype,
                                                           value.dtype):
@@ -925,7 +940,7 @@ def putmask(self, mask, new, align=True, inplace=False, axis=0,
             new = self.fill_value
 
         if self._can_hold_element(new):
-            _, new = self._try_coerce_args(new_values, new)
+            new = self._try_coerce_args(new)
 
             if transpose:
                 new_values = new_values.T
@@ -1127,7 +1142,8 @@ def _interpolate_with_fill(self, method='pad', axis=0, inplace=False,
                     return [self.copy()]
 
         values = self.values if inplace else self.values.copy()
-        values, fill_value = self._try_coerce_args(values, fill_value)
+        values = self._coerce_values(values)
+        fill_value = self._try_coerce_args(fill_value)
         values = missing.interpolate_2d(values, method=method, axis=axis,
                                         limit=limit, fill_value=fill_value,
                                         dtype=self.dtype)
@@ -1298,11 +1314,12 @@ def func(cond, values, other):
             if cond.ravel().all():
                 return values
 
-            values, other = self._try_coerce_args(values, other)
+            values = self._coerce_values(values)
+            other = self._try_coerce_args(other)
 
             try:
-                return self._try_coerce_result(expressions.where(
-                    cond, values, other))
+                fastres = expressions.where(cond, values, other)
+                return self._try_coerce_result(fastres)
             except Exception as detail:
                 if errors == 'raise':
                     raise TypeError(
@@ -1349,10 +1366,10 @@ def func(cond, values, other):
         result_blocks = []
         for m in [mask, ~mask]:
             if m.any():
-                r = self._try_cast_result(result.take(m.nonzero()[0],
-                                                      axis=axis))
-                result_blocks.append(
-                    self.make_block(r.T, placement=self.mgr_locs[m]))
+                taken = result.take(m.nonzero()[0], axis=axis)
+                r = self._try_cast_result(taken)
+                nb = self.make_block(r.T, placement=self.mgr_locs[m])
+                result_blocks.append(nb)
 
         return result_blocks
 
@@ -1423,7 +1440,7 @@ def quantile(self, qs, interpolation='linear', axis=0):
             values = values[None, :]
         else:
             values = self.get_values()
-            values, _ = self._try_coerce_args(values, values)
+            values = self._coerce_values(values)
 
         is_empty = values.shape[axis] == 0
         orig_scalar = not is_list_like(qs)
@@ -1579,7 +1596,8 @@ def putmask(self, mask, new, align=True, inplace=False, axis=0,
         # use block's copy logic.
         # .values may be an Index which does shallow copy by default
         new_values = self.values if inplace else self.copy().values
-        new_values, new = self._try_coerce_args(new_values, new)
+        new_values = self._coerce_values(new_values)
+        new = self._try_coerce_args(new)
 
         if isinstance(new, np.ndarray) and len(new) == len(mask):
             new = new[mask]
@@ -2120,25 +2138,25 @@ def _can_hold_element(self, element):
         return (is_integer(element) or isinstance(element, datetime) or
                 isna(element))
 
-    def _try_coerce_args(self, values, other):
+    def _coerce_values(self, values):
+        return values.view('i8')
+
+    def _try_coerce_args(self, other):
         """
-        Coerce values and other to dtype 'i8'. NaN and NaT convert to
+        Coerce other to dtype 'i8'. NaN and NaT convert to
         the smallest i8, and will correctly round-trip to NaT if converted
         back in _try_coerce_result. values is always ndarray-like, other
         may not be
 
         Parameters
         ----------
-        values : ndarray-like
         other : ndarray-like or scalar
 
         Returns
         -------
-        base-type values, base-type other
+        base-type other
         """
 
-        values = values.view('i8')
-
         if isinstance(other, bool):
             raise TypeError
         elif is_null_datetimelike(other):
@@ -2156,7 +2174,7 @@ def _try_coerce_args(self, values, other):
             # let higher levels handle
             raise TypeError(other)
 
-        return values, other
+        return other
 
     def _try_coerce_result(self, result):
         """ reverse of try_coerce_args """
@@ -2249,13 +2267,6 @@ def is_view(self):
         # check the ndarray values of the DatetimeIndex values
         return self.values._data.base is not None
 
-    def copy(self, deep=True):
-        """ copy constructor """
-        values = self.values
-        if deep:
-            values = values.copy()
-        return self.make_block_same_class(values)
-
     def get_values(self, dtype=None):
         """
         Returns an ndarray of values.
@@ -2305,21 +2316,22 @@ def _slice(self, slicer):
             return self.values[loc]
         return self.values[slicer]
 
-    def _try_coerce_args(self, values, other):
+    def _coerce_values(self, values):
+        # asi8 is a view, needs copy
+        return _block_shape(values.view("i8"), ndim=self.ndim)
+
+    def _try_coerce_args(self, other):
         """
         localize and return i8 for the values
 
         Parameters
         ----------
-        values : ndarray-like
         other : ndarray-like or scalar
 
         Returns
         -------
-        base-type values, base-type other
+        base-type other
         """
-        # asi8 is a view, needs copy
-        values = _block_shape(values.view("i8"), ndim=self.ndim)
 
         if isinstance(other, ABCSeries):
             other = self._holder(other)
@@ -2347,7 +2359,7 @@ def _try_coerce_args(self, values, other):
         else:
             raise TypeError(other)
 
-        return values, other
+        return other
 
     def _try_coerce_result(self, result):
         """ reverse of try_coerce_args """
@@ -2488,21 +2500,22 @@ def fillna(self, value, **kwargs):
             value = Timedelta(value, unit='s')
         return super().fillna(value, **kwargs)
 
-    def _try_coerce_args(self, values, other):
+    def _coerce_values(self, values):
+        return values.view('i8')
+
+    def _try_coerce_args(self, other):
         """
         Coerce values and other to int64, with null values converted to
         iNaT. values is always ndarray-like, other may not be
 
         Parameters
         ----------
-        values : ndarray-like
         other : ndarray-like or scalar
 
         Returns
         -------
-        base-type values, base-type other
+        base-type other
         """
-        values = values.view('i8')
 
         if isinstance(other, bool):
             raise TypeError
@@ -2517,7 +2530,7 @@ def _try_coerce_args(self, values, other):
             # let higher levels handle
             raise TypeError(other)
 
-        return values, other
+        return other
 
     def _try_coerce_result(self, result):
         """ reverse of try_coerce_args / try_operate """
@@ -2688,7 +2701,7 @@ def _maybe_downcast(self, blocks, downcast=None):
     def _can_hold_element(self, element):
         return True
 
-    def _try_coerce_args(self, values, other):
+    def _try_coerce_args(self, other):
         """ provide coercion to our input arguments """
 
         if isinstance(other, ABCDatetimeIndex):
@@ -2701,7 +2714,7 @@ def _try_coerce_args(self, values, other):
             # when falling back to ObjectBlock.where
             other = other.astype(object)
 
-        return values, other
+        return other
 
     def should_store(self, value):
         return not (issubclass(value.dtype.type,

pandas/tests/internals/test_internals.py

@@ -299,14 +299,14 @@ def test_try_coerce_arg(self):
         block = create_block('datetime', [0])
 
         # coerce None
-        none_coerced = block._try_coerce_args(block.values, None)[1]
+        none_coerced = block._try_coerce_args(None)
         assert pd.Timestamp(none_coerced) is pd.NaT
 
         # coerce different types of date bojects
         vals = (np.datetime64('2010-10-10'), datetime(2010, 10, 10),
                 date(2010, 10, 10))
         for val in vals:
-            coerced = block._try_coerce_args(block.values, val)[1]
+            coerced = block._try_coerce_args(val)
             assert np.int64 == type(coerced)
             assert pd.Timestamp('2010-10-10') == pd.Timestamp(coerced)