REF: uses_mask in group_any_all #52043
Conversation
| # so by Kleene logic the result is currently unknown | ||
| if out[lab, j] != flag_val: | ||
| out[lab, j] = -1 | ||
| result_mask[lab, j] = 1 |
There was a problem hiding this comment.
For any: If NA is encountered as first value in the group you are setting the mask to 1 here but you don't reset it if you find another value in the group that is not NA. You'll have to update the result_mask if you find another value.
There was a problem hiding this comment.
Oh OK. I missed that we are checking out[lab, j] != ... here as opposed to values[i, j] != .... Thanks.
| pp_kwargs["result_mask"] = result_mask | ||
|
|
||
| result = post_processing(result, inferences, **pp_kwargs) | ||
| result = post_processing(result, inferences, result_mask=result_mask) |
There was a problem hiding this comment.
You'll probably have to remove the flag is_nullable for the std post_processing function (the default is False) and check for result_mask is not None
phofl
left a comment
phofl
left a comment
There was a problem hiding this comment.
Thanks for tackling this, this was on my todo list as well
|
Your suggestion helped, green |
|
thx @jbrockmendel |
2 participants
any, all, and std go through GroupBy._get_cythonized_result instead of the more-standard WrappedCythonOp. I'm trying to refactor any/all to use the other path, and as a step toward that am trying to make group_any_all follow the same patterns as the other functions in libgroupby.
The implementation here looks to me like it should behave identically to the existing implementation, but it fails a bunch of tests (mostly in tests/groupby/test_any_all.py). Hoping @phofl can explain where I'm going wrong.