CLN: dont consolidate in indexing #34679
Conversation
jreback
added
the
Indexing
| @@ -1676,7 +1676,6 @@ def isetter(loc, v): | |||
| ser = v | |||
| else: | |||
| # set the item, possibly having a dtype change | |||
| ser._consolidate_inplace() | |||
There was a problem hiding this comment.
this one is a no-op since ser is a series (I think?), so can easily be removed
There was a problem hiding this comment.
This is my thought too
pandas/core/indexing.py
Outdated
| @@ -1788,7 +1787,6 @@ def isetter(loc, v): | |||
| self.obj._check_is_chained_assignment_possible() | |||
|
|
|||
| # actually do the set | |||
| self.obj._consolidate_inplace() | |||
There was a problem hiding this comment.
Removing this one will need a performance check, IMO, and also has potential API changes that might need investigation
There was a problem hiding this comment.
I think this consolidation turns out to be redundant with the self.obj._is_mixed_type check near the start of this method, since also does consolidate_inplace under the hood (though id prefer that it didnt)
and also has potential API changes that might need investigation
I've been thinking about this, since the same issue came up on the last call w/r/t always-view column-indexing. It isn't clear to me what type/amount of "investigation" would satisfy this concern. Do you have something in mind?
…rf-consolidate-less-indexing
|
reverted the non-consensus edit |
|
gentle ping. This is now just the 1-line edit |
|
lgtm |
No description provided.