-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: Add lazy copy to to_timestamp and to_period #50575
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5623,7 +5623,7 @@ def to_timestamp( | |
self, | ||
freq=None, | ||
how: Literal["s", "e", "start", "end"] = "start", | ||
copy: bool = True, | ||
copy: bool | None = None, | ||
) -> Series: | ||
""" | ||
Cast to DatetimeIndex of Timestamps, at *beginning* of period. | ||
|
@@ -5642,18 +5642,15 @@ def to_timestamp( | |
------- | ||
Series with DatetimeIndex | ||
""" | ||
new_values = self._values | ||
if copy: | ||
new_values = new_values.copy() | ||
new_obj = self.copy(deep=copy) | ||
|
||
if not isinstance(self.index, PeriodIndex): | ||
raise TypeError(f"unsupported Type {type(self.index).__name__}") | ||
new_index = self.index.to_timestamp(freq=freq, how=how) | ||
return self._constructor(new_values, index=new_index).__finalize__( | ||
self, method="to_timestamp" | ||
) | ||
setattr(new_obj, "index", new_index) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the reason not to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see this is how it is done for the |
||
return new_obj | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not fully sure, but we might want to keep the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see this is how it is done for the |
||
|
||
def to_period(self, freq: str | None = None, copy: bool = True) -> Series: | ||
def to_period(self, freq: str | None = None, copy: bool | None = None) -> Series: | ||
""" | ||
Convert Series from DatetimeIndex to PeriodIndex. | ||
|
||
|
@@ -5669,16 +5666,13 @@ def to_period(self, freq: str | None = None, copy: bool = True) -> Series: | |
Series | ||
Series with index converted to PeriodIndex. | ||
""" | ||
new_values = self._values | ||
if copy: | ||
new_values = new_values.copy() | ||
new_obj = self.copy(deep=copy) | ||
|
||
if not isinstance(self.index, DatetimeIndex): | ||
raise TypeError(f"unsupported Type {type(self.index).__name__}") | ||
new_index = self.index.to_period(freq=freq) | ||
return self._constructor(new_values, index=new_index).__finalize__( | ||
self, method="to_period" | ||
) | ||
setattr(new_obj, "index", new_index) | ||
return new_obj | ||
|
||
@overload | ||
def ffill( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,20 @@ | ||
from pandas import Series | ||
from pandas.core.arrays import BaseMaskedArray | ||
|
||
|
||
def get_array(df, col): | ||
def get_array(obj, col): | ||
""" | ||
Helper method to get array for a DataFrame column. | ||
Helper method to get array for a DataFrame column or a Series. | ||
|
||
Equivalent of df[col].values, but without going through normal getitem, | ||
which triggers tracking references / CoW (and we might be testing that | ||
this is done by some other operation). | ||
""" | ||
icol = df.columns.get_loc(col) | ||
if isinstance(obj, Series) and obj.name == col: | ||
return obj._values | ||
icol = obj.columns.get_loc(col) | ||
assert isinstance(icol, int) | ||
arr = df._get_column_array(icol) | ||
arr = obj._get_column_array(icol) | ||
if isinstance(arr, BaseMaskedArray): | ||
return arr._data | ||
return arr |
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.
Do this copy after the index isinstance 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.
Moved, kept the df as is to keep it consistent with the previous behavior