Skip to content
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

API/DEPR: replace "raise_conflict" with "errors" for df.update #23657

Merged
merged 6 commits into from
Nov 15, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ Deprecations
- The ``fastpath`` keyword of the different Index constructors is deprecated (:issue:`23110`).
- :meth:`Timestamp.tz_localize`, :meth:`DatetimeIndex.tz_localize`, and :meth:`Series.tz_localize` have deprecated the ``errors`` argument in favor of the ``nonexistent`` argument (:issue:`8917`)
- The class ``FrozenNDArray`` has been deprecated. When unpickling, ``FrozenNDArray`` will be unpickled to ``np.ndarray`` once this class is removed (:issue:`9031`)
- The methods :meth:`DataFrame.update` and :meth:`Panel.update` have deprecated the ``raise_conflict=False|True`` keyword in favor of ``errors='ignore'|'raise'`` (:issue:`23585`)
- Deprecated the `nthreads` keyword of :func:`pandas.read_feather` in favor of
`use_threads` to reflect the changes in pyarrow 0.11.0. (:issue:`23053`)
- :func:`pandas.read_excel` has deprecated accepting ``usecols`` as an integer. Please pass in a list of ints from 0 to ``usecols`` inclusive instead (:issue:`23527`)
Expand Down
18 changes: 12 additions & 6 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -5230,8 +5230,10 @@ def combiner(x, y):

return self.combine(other, combiner, overwrite=False)

@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,
raise_conflict=False):
errors='ignore'):
"""
Modify in place using non-NA values from another DataFrame.

Expand All @@ -5255,17 +5257,21 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
* False: only update values that are NA in
the original DataFrame.

filter_func : callable(1d-array) -> boolean 1d-array, optional
filter_func : callable(1d-array) -> bool 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`
Copy link
Contributor

Choose a reason for hiding this comment

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

versionchanged tag

both contain non-NA data in the same place.

Returns
-------
None : method directly changes calling object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added due to running docstring validation

Raises
------
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add an if for valid errors values (e.g. raise/error), and test for passing bad values in the tests themselves

ValueError
When `raise_conflict` is True and there's overlapping non-NA data.
When `errors='raise'` and there's overlapping non-NA data.

See Also
--------
Expand Down Expand Up @@ -5349,7 +5355,7 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
with np.errstate(all='ignore'):
mask = ~filter_func(this) | isna(that)
else:
if raise_conflict:
if errors == 'raise':
mask_this = notna(that)
mask_that = notna(this)
if any(mask_this & mask_that):
Expand Down
45 changes: 29 additions & 16 deletions pandas/core/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
create_block_manager_from_blocks)
from pandas.core.series import Series
from pandas.core.reshape.util import cartesian_product
from pandas.util._decorators import Appender, Substitution
from pandas.util._decorators import Appender, Substitution, deprecate_kwarg
from pandas.util._validators import validate_axis_style_args

_shared_doc_kwargs = dict(
Expand Down Expand Up @@ -1235,7 +1235,12 @@ def reindex(self, *args, **kwargs):
kwargs.update(axes)
kwargs.pop('axis', None)
kwargs.pop('labels', None)
return super(Panel, self).reindex(**kwargs)

with warnings.catch_warnings():
warnings.simplefilter("ignore", FutureWarning)
# do not warn about constructing Panel when reindexing
result = super(Panel, self).reindex(**kwargs)
return result
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from #23192:

I had no intention to add this warning filter here, but otherwise I cannot test the raise_conflict deprecation for Panel, as this warning was throwing me off from trying to catch the deprecation warning. If we don't care about testing that kwarg deprecation for Panel, I can revert this change here.


@Substitution(**_shared_doc_kwargs)
@Appender(NDFrame.rename.__doc__)
Expand Down Expand Up @@ -1377,25 +1382,33 @@ def join(self, other, how='left', lsuffix='', rsuffix=''):
return concat([self] + list(other), axis=0, join=how,
join_axes=join_axes, verify_integrity=True)

@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,
raise_conflict=False):
errors='ignore'):
"""
Modify Panel in place using non-NA values from passed
Panel, or object coercible to Panel. Aligns on items
Modify Panel in place using non-NA values from other Panel.

May also use object coercible to Panel. Will align on items.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ran the doctest validation, therefore changed more than just the errors text. But not essential to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is fine


Parameters
----------
other : Panel, or object coercible to Panel
join : How to join individual DataFrames
{'left', 'right', 'outer', 'inner'}, default 'left'
overwrite : boolean, default True
If True then overwrite values for common keys in the calling panel
filter_func : callable(1d-array) -> 1d-array<boolean>, default None
The object from which the caller will be udpated.
join : {'left', 'right', 'outer', 'inner'}, default 'left'
How individual DataFrames are joined.
overwrite : bool, default True
If True then overwrite values for common keys in the calling Panel.
filter_func : callable(1d-array) -> 1d-array<bool>, default None
Can choose to replace values other than NA. Return True for values
that should be updated
raise_conflict : bool
If True, will raise an error if a DataFrame and other both
contain data in the same place.
that should be updated.
errors : {'raise', 'ignore'}, default 'ignore'
If 'raise', will raise an error if a DataFrame and other both.

See Also
--------
DataFrame.update : Similar method for DataFrames.
dict.update : Similar method for dictionaries.
"""

if not isinstance(other, self._constructor):
Expand All @@ -1406,8 +1419,8 @@ def update(self, other, join='left', overwrite=True, filter_func=None,
other = other.reindex(**{axis_name: axis_values})

for frame in axis_values:
self[frame].update(other[frame], join, overwrite, filter_func,
raise_conflict)
self[frame].update(other[frame], join=join, overwrite=overwrite,
filter_func=filter_func, errors=errors)

def _get_join_index(self, other, how):
if how == 'left':
Expand Down
4 changes: 3 additions & 1 deletion pandas/tests/frame/test_combine_concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ def test_update_raise(self):
other = DataFrame([[2., nan],
[nan, 7]], index=[1, 3], columns=[1, 2])
with pytest.raises(ValueError, match="Data overlaps"):
df.update(other, raise_conflict=True)
df.update(other, errors='raise')
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move this to another test, e.g. test_update_deprecation

with tm.assert_produces_warning(FutureWarning):
df.update(other, raise_conflict=False)

def test_update_from_non_df(self):
d = {'a': Series([1, 2, 3, 4]), 'b': Series([5, 6, 7, 8])}
Expand Down
7 changes: 5 additions & 2 deletions pandas/tests/test_panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -2348,9 +2348,12 @@ def test_update_raise(self):
[[1.5, np.nan, 3.], [1.5, np.nan, 3.],
[1.5, np.nan, 3.],
[1.5, np.nan, 3.]]])
other = Panel([[[]]])

pytest.raises(Exception, pan.update, *(pan, ),
**{'raise_conflict': True})
with pytest.raises(ValueError, match='Data overlaps'):
pan.update(pan, errors='raise')
with tm.assert_produces_warning(FutureWarning):
pan.update(other, raise_conflict=True)

def test_all_any(self):
assert (self.panel.all(axis=0).values == nanall(
Expand Down