Skip to content

CoW warning mode: setting values into single column of DataFrame #56020

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

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

jorisvandenbossche
Copy link
Member

Next subtask of #56019.

This tackles mgr.column_setitem, which is used to set values into a single column (but not setting the full column, so like df.loc[idx, "col"] = ..)

@jorisvandenbossche
Copy link
Member Author

Green!

if not using_cow:
# Normally would need to do this before, but
# numpy only returns same array when round operation
# is no-op
# https://github.com/numpy/numpy/blob/486878b37fc7439a3b2b87747f50db9b62fea8eb/numpy/core/src/multiarray/calculation.c#L625-L636
values = values.copy()
else:
refs = self.refs
Copy link
Member Author

Choose a reason for hiding this comment

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

We were incorrectly adding forwarding refs always, even when we did make a copy. For CoW this doesn't cause anything wrong, but for the warning mode this gave false positive warnings.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah sorry for that, I didn't care much about non CoW when adding all of those

df = DataFrame({"a": [1, 2, 3], "b": 1, "c": 1})
view = df[:]

df["b"] = 100
arr = get_array(df, "a")
view = None # noqa: F841
df.iloc[0, 0] = 100
# TODO(CoW-warn) false positive?
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, comment explaining this is 3 lines below

@phofl phofl added this to the 2.2 milestone Nov 17, 2023
@phofl phofl merged commit de9815c into pandas-dev:main Nov 17, 2023
@phofl
Copy link
Member

phofl commented Nov 17, 2023

thx @jorisvandenbossche

@jorisvandenbossche jorisvandenbossche deleted the cow-warnings-column_setitem branch November 27, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants