REF: use fused types for groupby_helper #28886
Conversation
pandas/_libs/groupby_helper.pxi.in
Outdated
| @@ -548,9 +600,10 @@ def group_cummin(groupby_t[:, :] out, | |||
| def group_cummax(groupby_t[:, :] out, | |||
| groupby_t[:, :] values, | |||
| const int64_t[:] labels, | |||
| int ngroups, | |||
| int ngroups, | |||
There was a problem hiding this comment.
Looks like it was tabs, just fixed
| group_rank_float64 = group_rank["float64_t"] | ||
| group_rank_float32 = group_rank["float32_t"] | ||
| group_rank_int64 = group_rank["int64_t"] | ||
| # Note: we do not have a group_rank_object because that would require a |
There was a problem hiding this comment.
FWIW I don't think we really even want this #19560
There was a problem hiding this comment.
OK. The comment seemed appropriate since we are using the same fused type. Could add a reference to 19560 in the comment?
| for i in range(ncounts): | ||
| for j in range(K): | ||
| if nobs[i, j] == 0: | ||
| if rank_t is int64_t: |
There was a problem hiding this comment.
Do you see any harm in putting this condition in the object block as well? Not sure if this is covered by tests but could see someone mistakenly assuming that the gil and nogil blocks are identical when 0.30 gets released and missing this on refactor
There was a problem hiding this comment.
i like this idea, will do
Since we're probably going to wait for a while after 0.30 is released before bumping ourselves, that is longer than I'd prefer. As for using uint template generation in the interim, I did that before deciding that I am much more likely to screw up using tempita than I am using fused types. |
|
thanks @jbrockmendel we cannot wait for deps, who knows when they land |
There will be some nice cleanups we can do after we bump cython to 0.30 (which hasnt come out yet).
Also I think there is some na-checking code that we can share between the various fused-types functions after this.