Skip to content

API / CoW: constructing Series from Series creates lazy copy (with default copy=False) #49524

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

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Nov 4, 2022

xref #48998 (bullet point about constructors)

In the CoW proposal, the idea for constructors would be that those follow the lazy copy with CoW approach as well, just like other methods and indexing operations that create a new Series/DataFrame from another Series/DataFrame.
That was not yet implemented in the initial implementation, and this PR is adding it for the Series(series) case.

Currently, the way I implemented this is by making a shallow copy of the manager. This however also affects the default (non-CoW) behaviour -> see #49523.
Depending on whether we are OK with changing ("fixing"?) that behaviour in 2.0 or not, I can leave this as is, or otherwise put the copy behind a if get_cnofig("mode.copy_on_write") check.

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

lgtm, I would be ok with changing the behavior

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

LGTM too. Could use a whatsnew note

@github-actions github-actions bot added the Stale label Dec 17, 2022
@pandas-dev pandas-dev deleted a comment from github-actions bot Jan 2, 2023
@jorisvandenbossche jorisvandenbossche added this to the 2.0 milestone Jan 2, 2023
@@ -77,6 +77,13 @@ be set to ``"pyarrow"`` to return pyarrow-backed, nullable :class:`ArrowDtype` (
df_pyarrow = pd.read_csv(data, use_nullable_dtypes=True, engine="pyarrow")
df_pyarrow.dtypes

Copy on write improvements
^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member Author

Choose a reason for hiding this comment

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

Added a new section like is being added in #50471. We can consolidate those different blurbs later on in a coherent whatsnew.

@jorisvandenbossche
Copy link
Member Author

I updated this to put the actual code change behind a "if CoW" check (so this PR only changes something for the case of CoW enabled), and will do a separate PR to address #49523 (and for both Series and DataFrame).

@jorisvandenbossche
Copy link
Member Author

will do a separate PR to address #49523 (and for both Series and DataFrame).

Opened #50539 for this

@jorisvandenbossche jorisvandenbossche merged commit 4ff1f3e into pandas-dev:main Jan 13, 2023
@jorisvandenbossche jorisvandenbossche deleted the cow-constructor-from-pandas branch January 13, 2023 19:32
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.

3 participants