-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG-CoW: DataFrame constructed from Series not respecting CoW #52031
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
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.
Looks good!
@@ -291,7 +293,20 @@ def ndarray_to_mgr( | |||
if values.ndim == 1: | |||
values = values.reshape(-1, 1) | |||
|
|||
elif isinstance(values, (np.ndarray, ExtensionArray, ABCSeries, Index)): | |||
elif isinstance(values, ABCSeries): |
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.
Should this path handle both Series and Index?
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.
Ah, I see you indicated to do that in a follow-up (although I would assume it's "just" adding Index to this check?)
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.
Yeah I think so, but would mean adding more tests and increasing the diff. Separate would probably keep it a bit simpler
|
||
def test_dataframe_from_series_infer_datetime(using_copy_on_write): | ||
ser = Series([Timestamp("2019-12-31"), Timestamp("2020-12-31")], dtype=object) | ||
df = DataFrame(ser) |
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.
We should stop inferring this, but that's another issue ;)
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.
😅
# Conflicts: # doc/source/whatsnew/v2.0.0.rst # pandas/tests/copy_view/test_constructors.py
I'll merge this to get it into 2.0 and to make the follow up |
…om Series not respecting CoW) (#52274) Backport PR #52031: BUG-CoW: DataFrame constructed from Series not respecting CoW Co-authored-by: Patrick Hoefler <[email protected]>
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.right now we back the DataFrame by a read-only array, before the read-only pr this did not respect CoW.
Follow-up will be to do the same for Index.