-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REF: Remove sanitize_column from _iset_item #51702
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/frame.py
Outdated
arraylike = self._sanitize_column(value) | ||
self._iset_item_mgr(loc, arraylike, inplace=True) | ||
def _iset_item(self, loc: int, value: Series) -> None: | ||
self._iset_item_mgr(loc, value._values, inplace=True) |
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 think this is OK only because iset_item is called only from replace_columnwise which guarantees no reindex is necessary. can you add a comment to that effect?
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.
Added a comment, we have to keep the copy until reference tracking for setitem works properly (meaning the value that is set into the DataFrame).
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.
LGTM
BTW one of your other recent PRs that avoided passing DataFrame to sanitize_column might make it so that the allow_2d=True passed there could be unnecessary? |
I think we could still get there with 2D arrays, not sure though. Would have to check |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.this should work ootb. Don't think we need a copy here