-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Conversation
pandas/core/series.py
Outdated
| new_values = self._values | ||
| if copy: | ||
| new_values = new_values.copy() | ||
| new_obj = self.copy(deep=copy) |
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
| self, method="to_timestamp" | ||
| ) | ||
| setattr(new_obj, "index", new_index) | ||
| return new_obj |
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.
I am not fully sure, but we might want to keep the __finalize__ call? (although of course not a new object was constructed here, that already happened now in the copy() call)
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 this is how it is done for the DataFrame implementation, so that's probably OK then
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Is the reason not to use new_obj.index = new_index for performance?
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 this is how it is done for the DataFrame implementation
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.