BUG: passing DataFrame to make_block silently raises #28275
Conversation
|
Hmm so you are saying this improves mixed block performance where some can be cythonized right? This adds a little bit of logic so would be good to see that measured somehow to understand if it’s worth it |
I'm having a hard time cooking up an example, but roughly what I have in mind is a case with
In such a case we would end up falling back to the non-cython implementations for all blocks instead of just the one relevant EA block. |
| try: | ||
| result = s.aggregate(lambda x: alt(x, axis=self.axis)) | ||
| except TypeError: | ||
| # we may have an exception in trying to aggregate | ||
| # continue and exclude the block | ||
| deleted_items.append(locs) | ||
| continue | ||
|
|
||
| if is_object_dtype(block.dtype) and how in ["prod", "cumprod", "sum"]: |
There was a problem hiding this comment.
a way around is to have .aggregate different exceptions for flow control (make some private ones that inherit from NotImplementedError); we do this in the cython aggregator now (well we raise NotImplementedError)
There was a problem hiding this comment.
yah, distinguishing between our NotImplementedError vs numpy's is basically what #28198 does (albeit that is only in one place). I think the groupby catching/raising is complicated enough that we need to do a few passes clearing up what gets caught where, then go through to do the catching in fewer places
|
Closing to clear the queue. Will revisit. |
ATM in the
except NotImplementedError:branch of_cython_agg_blocks, we take a difference approach to the operation. if that approach succeeds, we end up passing a DataFrame tomake_blockon L189, which will raiseValueError. As a result, we'll end up falling back to python-space for the entire operation, which presumably entails a performance hit.This fixes the incorrect passing of DataFrame, but the fix is kind of kludgy. Suggestions welcome on how to improve it.