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

ENH/CoW: use lazy copy in set_axis method #49600

Merged
merged 7 commits into from
Dec 28, 2022

Conversation

ntachukwu
Copy link
Contributor

Copy-on-Write in set_axis

pandas/core/frame.py Show resolved Hide resolved
pandas/tests/frame/methods/test_set_axis.py Outdated Show resolved Hide resolved
pandas/tests/frame/methods/test_set_axis.py Outdated Show resolved Hide resolved
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

no objections, over to @jorisvandenbossche

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Looks good! Just the comment to add a variant of the test for Series as well

assert not np.shares_memory(get_array(df2, "a"), get_array(df, "a"))

# mutating df2 triggers a copy-on-write for that column / block
df2.iloc[0, 1] = 0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
df2.iloc[0, 1] = 0
df2.iloc[0, 0] = 0

(just to keep it as simple as possible, for this test there is no reason to not take the first value/column)

pandas/tests/copy_view/test_methods.py Show resolved Hide resolved
@jorisvandenbossche
Copy link
Member

And you will also have to merge the main branch, to resolve the conflict from merging your other PR

@jorisvandenbossche
Copy link
Member

@Th3nn3ss small reminder in case you have time to update for the minor comments above

@ntachukwu
Copy link
Contributor Author

sorry about the delay @jorisvandenbossche I was preoccupied with my research project. I have made some changes. Thanks for taking a look at my PR.

@ntachukwu ntachukwu requested review from phofl and removed request for jorisvandenbossche December 28, 2022 15:00
@phofl phofl added this to the 2.0 milestone Dec 28, 2022
@phofl phofl merged commit 0641183 into pandas-dev:main Dec 28, 2022
@phofl
Copy link
Member

phofl commented Dec 28, 2022

Thx @Th3nn3ss

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.

5 participants