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. |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
if you have a test for this would be best, but if you can revise sligthly can push thru
| 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
| # 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 |
3 participants
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.