Skip to content

TST: Add test for CoW in pop #50569

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 1 commit into from
Jan 5, 2023
Merged

Conversation

phofl
Copy link
Member

@phofl phofl commented Jan 4, 2023

cc @jorisvandenbossche
No change in behavior but confirming the behavior

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!


if using_copy_on_write:
result.iloc[0] = 0
assert not np.shares_memory(result.values, get_array(view_original, "a"))
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
assert not np.shares_memory(result.values, get_array(view_original, "a"))
assert not np.shares_memory(result.values, get_array(view_original, "a"))
assert df.iloc[0, 0] == 1

To ensure the original wasn't modified?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, no, nevermind. pop of course removed that column from df, so we don't have to check it there, and that's the wholeponit of having this view_original (which is already checked below that it is still equal to the original dataframe)

@jorisvandenbossche jorisvandenbossche changed the title TST: Add test for cow in pop TST: Add test for CoW in pop Jan 5, 2023
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.

Failures are unrelated sqlalchemy failures

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