BUG raise pdep6 warning for loc full setter #56146
Conversation
mroeschke
added
the
PDEP6-related
|
thanks for your reviews! have updated |
doc/source/whatsnew/v2.2.0.rst
Outdated
| @@ -584,6 +584,7 @@ Conversion | |||
| - Bug in :meth:`DataFrame.astype` when called with ``str`` on unpickled array - the array might change in-place (:issue:`54654`) | |||
| - Bug in :meth:`DataFrame.astype` where ``errors="ignore"`` had no effect for extension types (:issue:`54654`) | |||
| - Bug in :meth:`Series.convert_dtypes` not converting all NA column to ``null[pyarrow]`` (:issue:`55346`) | |||
| - Bug in ``DataFrame.loc`` was not throwing "incompatible dtype warning" (see PDEP6) when assigning a ``Series`` with a different dtype using a full column setter (e.g. ``df.loc[:, 'a'] = incompatible_value``) (:issue:`39584`) | |||
There was a problem hiding this comment.
should "PDEP6" here get some type of colons or backticks or something to become a link? (i dont care that much)
| @@ -498,6 +498,9 @@ def coerce_to_target_dtype(self, other, warn_on_upcast: bool = False) -> Block: | |||
| and is_integer_dtype(self.values.dtype) | |||
| and isna(other) | |||
| and other is not NaT | |||
| and not ( | |||
| isinstance(other, (np.datetime64, np.timedelta64)) and np.isnat(other) | |||
There was a problem hiding this comment.
i think the isnat here is unnecessary since we already have isna(other) above
There was a problem hiding this comment.
the idea of the this if statement is to check whether self is of integer type, and other is of a type that it will be possible, in the future, to set into self without having to coerce self
This is the case if other is nan or NA, but presumably we still want to raise if someone tries setting NaT? If so, then this check if looking for cases when isna(other) is True but other was not NaT - that's why I don't think the isnat check is unnecessary
pandas/tests/groupby/test_groupby.py
Outdated
| @@ -199,7 +199,7 @@ def f_1(grp): | |||
| with tm.assert_produces_warning(FutureWarning, match=msg): | |||
| result = df.groupby("A").apply(f_1)[["B"]] | |||
| e = expected.copy() | |||
| e.loc["Tiger"] = np.nan | |||
| e["B"] = [3, 5, float("nan")] | |||
There was a problem hiding this comment.
why is this changing? any reason for float('nan') instead of np.nan?
There was a problem hiding this comment.
indeed, it didn't need to change, thanks
| @@ -491,7 +491,7 @@ def _check_setitem_invalid(self, ser, invalid, indexer, warn): | |||
| np.datetime64("NaT"), | |||
| np.timedelta64("NaT"), | |||
| ] | |||
| _indexers = [0, [0], slice(0, 1), [True, False, False]] | |||
| _indexers = [0, [0], slice(0, 1), [True, False, False], slice(None, None, None)] | |||
There was a problem hiding this comment.
this looks similar to the DataFrame test above. lets try to de-duplicate it one of these days
|
thx @MarcoGorelli |
|
@meeseeksdev backport 2.2.x |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
|
@MarcoGorelli could you take care of the backport? |
|
Yup will do in the morning, thanks! |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.