-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Fixed bug when creating new column with missing values when setting a single string value #56321
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
Conversation
00bed88
to
b3774b1
Compare
doc/source/whatsnew/v2.1.4.rst
Outdated
@@ -13,6 +13,7 @@ including other versions of pandas. | |||
|
|||
Fixed regressions | |||
~~~~~~~~~~~~~~~~~ | |||
- Fixed bug when creating new column with missing values when setting a single string value (:issue:`56204`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put this into the bug column?
def test_adding_new_conditional_column_with_string() -> None: | ||
# https://github.com/pandas-dev/pandas/issues/56204 | ||
df = DataFrame({"a": [1, 2], "b": [3, 4]}) | ||
df.loc[lambda x: x.a == 1, "c"] = "1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it important that this be a lambda?
tm.assert_frame_equal(df, expected) | ||
|
||
|
||
def test_adding_new_conditional_column_with_infer_string() -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make this one test and parametrize? in 3.0 we'll only want to end up with one test here right?
thanks for your reviews! have updated |
Just tracked down an old branch where past-me was trying to get rid of infer_fill_value. The only recoverable bit of that branch looks like it would go just before L1880 in indexing.py
would that solve the issue here robustly? i like past-me's idea of using sanitize_array rather than yet-another constructor-esque function |
That would also avoid the warning when expanding column-wise: main: In [1]: df = pd.DataFrame({'a': [.1, .2], 'b': [.3, .4]})
In [2]: df.loc[1, 'c'] = True
<ipython-input-2-d3d953ae007a>:1: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise in a future error of pandas. Value 'True' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
df.loc[1, 'c'] = True
In [3]: df
Out[3]:
a b c
0 0.1 0.3 NaN
1 0.2 0.4 True
In [4]: df.dtypes
Out[4]:
a float64
b float64
c object
dtype: object here: In [1]: df = pd.DataFrame({'a': [.1, .2], 'b': [.3, .4]})
In [2]: df.loc[1, 'c'] = True
In [3]: df
Out[3]:
a b c
0 0.1 0.3 NaN
1 0.2 0.4 True
In [4]: df.dtypes
Out[4]:
a float64
b float64
c object
dtype: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
doc/source/whatsnew/v2.1.4.rst
Outdated
@@ -35,6 +35,7 @@ Bug fixes | |||
- Fixed bug in :meth:`Series.reset_index` not preserving object dtype when ``infer_string`` is set (:issue:`56160`) | |||
- Fixed bug in :meth:`Series.str.split` and :meth:`Series.str.rsplit` when ``pat=None`` for :class:`ArrowDtype` with ``pyarrow.string`` (:issue:`56271`) | |||
- Fixed bug in :meth:`Series.str.translate` losing object dtype when string option is set (:issue:`56152`) | |||
- Fixed bug when creating new column with missing values when setting a single string value (:issue:`56204`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this 2.2.0.rst
?
thanks for your reviews! merging then |
…ing a single string value (pandas-dev#56321)
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.