BUG: avoid incorrectly inheriting result #27445
Conversation
|
Hmm not immediately apparent here but what problems does this fix? |
You're right that ATM this is a solution in search of a problem. This becomes necessary in the branch that is the follow-up to #27428 (which is itself a follow-up to #27411). I can close and re-open when it becomes actually necessary, but am hoping I can find a useful example in the interim, since the failure mode is clear. |
| @@ -143,8 +143,12 @@ def _cython_agg_blocks(self, how, alt=None, numeric_only=True, min_count=-1): | |||
| new_blocks = [] | |||
| new_items = [] | |||
| deleted_items = [] | |||
| no_result = object() | |||
There was a problem hiding this comment.
why don't you just set result = None outside the try instead as its more clear
There was a problem hiding this comment.
because I don't know for sure that None won't be the correct result produced on line 154
| @@ -171,15 +175,16 @@ def _cython_agg_blocks(self, how, alt=None, numeric_only=True, min_count=-1): | |||
| except TypeError: | |||
| # we may have an exception in trying to aggregate | |||
| # continue and exclude the block | |||
| pass | |||
| deleted_items.append(locs) | |||
There was a problem hiding this comment.
do we actually use the deleted_items?
Yah, I'd rather leave this open for now until it becomes necessary (following #27428) or I can come up with a test. |
not averse but if we can't break the current code then maybe wait on this. |
|
Closing in favor of #27567, which includes this as a subset |
This has caused problems in a number of local branches, but I don't have a good test case on master. Suggestions welcome. Still though, the failure mode is pretty clear, so this fixes it.