REF: move logic of 'block manager axis' into the BlockManager #40075
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
+1 |
|
yep +1 on moving this to the BM. |
|
I also added the change for Other uses cases are eg in |
jreback
added
Internals
jreback
requested changes
Mar 4, 2021
| @@ -385,7 +385,10 @@ def apply( | |||
|
|
|||
| return type(self)(result_arrays, new_axes) | |||
|
|
|||
| def apply_with_block(self: T, f, align_keys=None, **kwargs) -> T: | |||
| def apply_with_block(self: T, f, align_keys=None, swap_axis=True, **kwargs) -> T: | |||
| # switch axis to follow BlockManager logic | |||
There was a problem hiding this comment.
is this needed now that you have a _normalize_axis?
There was a problem hiding this comment.
Yes, it is still needed here because this is the function that does the "use block method as fallback" (and the block method is still using its inverted axis logic).
(but I am actively removing the usage of apply_with_block, like #40196)
|
can you merge master |
|
once more merge master pls |
jreback
approved these changes
Mar 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@jreback @jbrockmendel what are your thoughts about doing this?
Currently the 0/1 -> 1/0 axis switch for the BlockManager happens on the DataFrame side, while I think it would be a bit cleaner to let this be the responsibility of the BlockManager method being called.
For illustration, the diff of this PR is such a change for the
diff()method: instead of using_get_block_manager_axisin frame.py, it's theBlockManager.diff()method that takes care of this (so just moving it down a level).