Review (jreback)

Author: h-vetinari

doc/source/whatsnew/v0.24.0.rst

@@ -281,7 +281,8 @@ Other Enhancements
   all instances of ``set`` will not be considered "list-like" anymore (:issue:`23061`)
 - :meth:`Index.to_frame` now supports overriding column name(s) (:issue:`22580`).
 - New attribute :attr:`__git_version__` will return git commit sha of current build (:issue:`21295`).
-- :meth:`Series.update` now supports the same keywords and functionality as :meth:`DataFrame.update` (:issue:`22358`)
+- :meth:`Series.update` now supports the same keywords and functionality as :meth:`DataFrame.update`.
+  In particular, it has gained the keywords ``overwrite``, ``filter_func`` and ``errors`` (:issue:`22358`)
 - Compatibility with Matplotlib 3.0 (:issue:`22790`).
 - Added :meth:`Interval.overlaps`, :meth:`IntervalArray.overlaps`, and :meth:`IntervalIndex.overlaps` for determining overlaps between interval-like objects (:issue:`21998`)
 - :func:`~DataFrame.to_parquet` now supports writing a ``DataFrame`` as a directory of parquet files partitioned by a subset of the columns when ``engine = 'pyarrow'`` (:issue:`23283`)

pandas/core/frame.py

@@ -5203,6 +5203,14 @@ def combiner(x, y):
 
         return self.combine(other, combiner, overwrite=False)
 
+    @Appender(NDFrame.update.__doc__)
+    @deprecate_kwarg(old_arg_name='raise_conflict', new_arg_name='errors',
+                     mapping={False: 'ignore', True: 'raise'})
+    def update(self, other, join='left', overwrite=True, filter_func=None,
+               errors='ignore'):
+        super(DataFrame, self).update(other, join=join, overwrite=overwrite,
+                                      filter_func=filter_func, errors=errors)
+
     # ----------------------------------------------------------------------
     # Data reshaping
 

pandas/core/generic.py

@@ -101,32 +101,6 @@ def _single_replace(self, to_replace, method, inplace, limit):
     return result
 
 
-def _update_column(this, that, overwrite=True, filter_func=None,
-                   raise_conflict=False):
-    import pandas.core.computation.expressions as expressions
-
-    if filter_func is not None:
-        with np.errstate(all='ignore'):
-            mask = ~filter_func(this) | isna(that)
-    else:
-        if raise_conflict:
-            mask_this = notna(that)
-            mask_that = notna(this)
-            if any(mask_this & mask_that):
-                raise ValueError("Data overlaps.")
-
-        if overwrite:
-            mask = isna(that)
-        else:
-            mask = notna(this)
-
-    # don't overwrite columns unnecessarily
-    if mask.all():
-        return None
-
-    return expressions.where(mask, this, that)
-
-
 class NDFrame(PandasObject, SelectionMixin):
     """
     N-dimensional analogue of DataFrame. Store multi-dimensional in a
@@ -4200,7 +4174,7 @@ def _reindex_with_indexers(self, reindexers, fill_value=None, copy=False,
         return self._constructor(new_data).__finalize__(self)
 
     def update(self, other, join='left', overwrite=True, filter_func=None,
-               raise_conflict=False):
+               errors='ignore'):
         """
         Modify in place using non-NA values from another DataFrame.
 
@@ -4227,8 +4201,8 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
         filter_func : callable(1d-array) -> boolean 1d-array, optional
             Can choose to replace values other than NA. Return True for values
             that should be updated.
-        raise_conflict : bool, default False
-            If True, will raise a ValueError if the DataFrame and `other`
+        errors : {'raise', 'ignore'}, default 'ignore'
+            If 'raise', will raise a ValueError if the DataFrame and `other`
             both contain non-NA data in the same place.
 
         Raises
@@ -4317,13 +4291,15 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
             other = other.reindex_like(self)
             this = self.values
             that = other.values
-            updated = _update_column(this, that, overwrite=overwrite,
-                                     filter_func=filter_func,
-                                     raise_conflict=raise_conflict)
-            if updated is None:
-                # don't overwrite Series unnecessarily
-                return
-            self._data._block.values = updated
+
+            # missing.update_array returns an np.ndarray
+            updated_values = missing.update_array(this, that,
+                                                  overwrite=overwrite,
+                                                  filter_func=filter_func,
+                                                  errors=errors)
+            # don't overwrite unnecessarily
+            if updated_values is not None:
+                self._update_inplace(Series(updated_values, index=self.index))
         else:  # DataFrame
             if not isinstance(other, ABCDataFrame):
                 other = DataFrame(other)
@@ -4334,13 +4310,12 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
                 this = self[col].values
                 that = other[col].values
 
-                updated = _update_column(this, that, overwrite=overwrite,
-                                         filter_func=filter_func,
-                                         raise_conflict=raise_conflict)
-                # don't overwrite columns unnecessarily
-                if updated is None:
-                    continue
-                self[col] = updated
+                updated = missing.update_array(this, that, overwrite=overwrite,
+                                           filter_func=filter_func,
+                                           errors=errors)
+                # don't overwrite unnecessarily
+                if updated is not None:
+                    self[col] = updated
 
     def filter(self, items=None, like=None, regex=None, axis=None):
         """

pandas/core/missing.py

@@ -21,7 +21,7 @@
     ensure_float64)
 
 from pandas.core.dtypes.cast import infer_dtype_from_array
-from pandas.core.dtypes.missing import isna
+from pandas.core.dtypes.missing import isna, notna
 
 
 def mask_missing(arr, values_to_mask):
@@ -75,6 +75,70 @@ def mask_missing(arr, values_to_mask):
     return mask
 
 
+def update_array(this, that, overwrite=True, filter_func=None,
+                 errors='ignore'):
+    """
+    Update one array with non-NA values from another array.
+
+    Parameters
+    ----------
+    this : np.ndarray (one-dimensional)
+        The array being updated.
+    that : np.ndarray (one-dimensional)
+        The array being used to update.
+    overwrite : bool, default True
+        How to handle non-NA values for overlapping keys:
+
+        * True: overwrite original array's values with values from `that`.
+        * False: only update values that are NA in `this`.
+
+    filter_func : callable(1d-array) -> boolean 1d-array, optional
+        Can choose to replace values other than NA. Return True for values
+        that should be updated.
+    errors : {'raise', 'ignore'}, default 'ignore'
+        If 'raise', will raise a ValueError if `this` and `that` both contain
+        non-NA data in the same place.
+
+    Raises
+    ------
+    ValueError
+        When `errors='raise'` and there's overlapping non-NA data.
+
+    Returns
+    -------
+    updated : np.ndarray (one-dimensional) or None
+        The updated array. Return None if `this` remains unchanged
+
+    See Also
+    --------
+    Series.update : Similar method for `Series`.
+    DataFrame.update : Similar method for `DataFrame`.
+    dict.update : Similar method for `dict`.
+    """
+    import pandas.core.computation.expressions as expressions
+
+    if filter_func is not None:
+        with np.errstate(all='ignore'):
+            mask = ~filter_func(this) | isna(that)
+    else:
+        if errors == 'raise':
+            mask_this = notna(that)
+            mask_that = notna(this)
+            if any(mask_this & mask_that):
+                raise ValueError("Data overlaps.")
+
+        if overwrite:
+            mask = isna(that)
+        else:
+            mask = notna(this)
+
+    # don't overwrite columns unnecessarily
+    if mask.all():
+        return None
+
+    return expressions.where(mask, this, that)
+
+
 def clean_fill_method(method, allow_nearest=False):
     # asfreq is compat for resampling
     if method in [None, 'asfreq']:

pandas/core/series.py

@@ -2386,7 +2386,7 @@ def combine_first(self, other):
         return this.where(notna(this), other)
 
     def update(self, other, join='left', overwrite=True, filter_func=None,
-               raise_conflict=False):
+               errors='ignore'):
         """
         Modify Series in place using non-NA values from passed Series.
 
@@ -2400,6 +2400,8 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
         join : {'left'}, default 'left'
             Only left join is implemented, keeping the index and columns of the
             original object.
+
+            .. versionadded:: 0.24.0
         overwrite : bool, default True
             How to handle non-NA values for overlapping keys:
 
@@ -2408,13 +2410,18 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
             * False: only update values that are NA in
               the original DataFrame.
 
+            .. versionadded:: 0.24.0
         filter_func : callable(1d-array) -> boolean 1d-array, optional
             Can choose to replace values other than NA. Return True for values
             that should be updated.
-        raise_conflict : bool, default False
-            If True, will raise a ValueError if the DataFrame and `other`
+
+            .. versionadded:: 0.24.0
+        errors : {'raise', 'ignore'}, default 'ignore'
+            If 'raise', will raise a ValueError if the DataFrame and `other`
             both contain non-NA data in the same place.
 
+            .. versionadded:: 0.24.0
+
         See Also
         --------
         DataFrame.update : Similar method for `DataFrame`.
@@ -2459,7 +2466,7 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
         """
         super(Series, self).update(other, join=join, overwrite=overwrite,
                                    filter_func=filter_func,
-                                   raise_conflict=raise_conflict)
+                                   errors=errors)
 
     # ----------------------------------------------------------------------
     # Reindexing, sorting