-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: Avoid post-processing in blockwise op #35356
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
|
||
no_result = object() | ||
|
||
def cast_result_block(result, block: "Block", how: str) -> "Block": |
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.
can you put this function elsewhere (module level somewhere) and import, in-line functions make much harder to grok.
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.
this likely actually belongs as a method on block itself no?
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 dont think this merits a Block method. There will be further refactoring coming up with the goal of having this iteration-over-blocks go through a BlockManager method
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.
@jreback is this a deal-breaker? id like to postpone this decision until after implementing the BlockManager method that this/apply/ewm eventually 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.
ok sounds good
Small step towards #34714
cc @TomAugspurger IIRC you implemented split_frames/split_items, which this removes