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

WARN: Remove false positive warning for iloc inplaceness #48397

Merged
merged 22 commits into from Sep 16, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pandas/_testing/__init__.py
Expand Up @@ -459,7 +459,8 @@ def all_timeseries_index_generator(k: int = 10) -> Iterable[Index]:
def make_rand_series(name=None, dtype=np.float64) -> Series:
index = makeStringIndex(_N)
data = np.random.randn(_N)
data = data.astype(dtype, copy=False)
with np.errstate(invalid="ignore"):
data = data.astype(dtype, copy=False)
return Series(data, index=index, name=name)


Expand Down
5 changes: 4 additions & 1 deletion pandas/core/dtypes/cast.py
Expand Up @@ -1975,7 +1975,10 @@ def np_can_hold_element(dtype: np.dtype, element: Any) -> Any:
if tipo.kind not in ["i", "u"]:
if isinstance(element, np.ndarray) and element.dtype.kind == "f":
# If all can be losslessly cast to integers, then we can hold them
casted = element.astype(dtype)
with np.errstate(invalid="ignore"):
mroeschke marked this conversation as resolved.
Show resolved Hide resolved
# We check afterwards if cast was losslessly, so no need to show
# the warning
casted = element.astype(dtype)
comp = casted == element
if comp.all():
# Return the casted values bc they can be passed to
Expand Down
16 changes: 6 additions & 10 deletions pandas/core/indexing.py
Expand Up @@ -1990,21 +1990,17 @@ def _setitem_single_column(self, loc: int, value, plane_indexer) -> None:
self.obj._clear_item_cache()
return

self.obj._iset_item(loc, value)

# We will not operate in-place, but will attempt to in the future.
# To determine whether we need to issue a FutureWarning, see if the
# setting in-place would work, i.e. behavior will change.
if isinstance(value, ABCSeries):
warn = can_hold_element(orig_values, value._values)
else:
warn = can_hold_element(orig_values, value)

# Don't issue the warning yet, as we can still trim a few cases where
# behavior will not change.

self.obj._iset_item(loc, value)
new_values = self.obj._get_column_array(loc)

if warn:
new_values = self.obj._get_column_array(loc)
if can_hold_element(orig_values, new_values):
# Don't issue the warning yet, as we can still trim a few cases where
# behavior will not change.

if (
isinstance(new_values, np.ndarray)
Expand Down
3 changes: 2 additions & 1 deletion pandas/tests/frame/indexing/test_indexing.py
Expand Up @@ -1326,7 +1326,8 @@ def test_loc_expand_empty_frame_keep_midx_names(self):
def test_loc_setitem_rhs_frame(self, idxr, val):
# GH#47578
df = DataFrame({"a": [1, 2]})
df.loc[:, "a"] = DataFrame({"a": [val, 11]}, index=[1, 2])
with tm.assert_produces_warning(None):
df.loc[:, idxr] = DataFrame({"a": [val, 11]}, index=[1, 2])
expected = DataFrame({"a": [np.nan, val]})
tm.assert_frame_equal(df, expected)

Expand Down
10 changes: 7 additions & 3 deletions pandas/tests/frame/methods/test_diff.py
Expand Up @@ -91,9 +91,13 @@ def test_diff_datetime_with_nat_zero_periods(self, tz):

df[1] = ser.copy()

msg = "will attempt to set the values inplace instead"
with tm.assert_produces_warning(FutureWarning, match=msg):
df.iloc[:, 0] = pd.NaT
if tz is None:
msg = "will attempt to set the values inplace instead"
with tm.assert_produces_warning(FutureWarning, match=msg):
df.iloc[:, 0] = pd.NaT
Copy link
Member

Choose a reason for hiding this comment

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

@phofl i think i misunderstood earlier when i said this should warn. i was under the impression that the RHS was pd.Series([pd.NaT]*N, dtype="M8[ns]"), not the scalar pd.NaT. With the scalar, this should not warn.

Copy link
Member

Choose a reason for hiding this comment

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

that is an existing problem that i think is orthogonal to this PR, so no need to hold up on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep agreed, the NaT gets cast to an all NaT Series without a timezone, hence this seems equal

Copy link
Member

Choose a reason for hiding this comment

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

ideal then would be to assert_warns(None) unconditionally and xfail the tzaware case

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

else:
with tm.assert_produces_warning(None):
df.iloc[:, 0] = pd.NaT

expected = df - df
assert expected[0].isna().all()
Expand Down
6 changes: 5 additions & 1 deletion pandas/tests/frame/test_nonunique_indexes.py
@@ -1,6 +1,8 @@
import numpy as np
import pytest

from pandas.compat import is_platform_windows

import pandas as pd
from pandas import (
DataFrame,
Expand Down Expand Up @@ -320,7 +322,9 @@ def test_dup_columns_across_dtype(self):

def test_set_value_by_index(self, using_array_manager):
# See gh-12344
warn = FutureWarning if using_array_manager else None
warn = (
FutureWarning if using_array_manager and not is_platform_windows() else None
)
jbrockmendel marked this conversation as resolved.
Show resolved Hide resolved
msg = "will attempt to set the values inplace"

df = DataFrame(np.arange(9).reshape(3, 3).T)
Expand Down