-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: remove placement kwarg from Block.concat_same_type #33123
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 to me
pandas/core/internals/blocks.py
Outdated
values = self._concatenator( | ||
[blk.values for blk in to_concat], axis=self.ndim - 1 | ||
) | ||
values = self._holder._concat_same_type([blk.values for blk in to_concat]) |
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 know the _concatenator
-> _holder._concat_same_type
change is right now equivalent. But I would maybe leave it as is for now, because actually Categorical._concat_same_type
is not implementing the spec of returning its own type / this problem is something we need to fix in general for ExtensionArrays.
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.
Either way I guess. I'm hopeful Block.concat_same_type will be gone by then
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.
reverted
return self.make_block_same_class( | ||
values, placement=placement or slice(0, len(values), 1) | ||
) | ||
placement = self.mgr_locs if self.ndim == 2 else slice(len(values)) |
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.
you can remove the placement arg from make_block_same_class I think (e.g. do this check internally), but for a followon
On the path to getting concat logic out of internals