REF: use fused types in algos_rank_helper #29044
Conversation
| order = (values, mask) | ||
| if rank_t is not uint64_t: | ||
| if ascending ^ (na_option == 'top'): | ||
| if rank_t is object: |
There was a problem hiding this comment.
Not necessarily in scope for this but would be nice if there was a better way to do this via fused types (a dict or inline func?)
There was a problem hiding this comment.
Yah I've been giving that some thought. I think we can do that with something like:
cdef inline rank_t get_thing(rank_t val): # <-- totally ignore `val`, just care about rank_t
if rank_t is object: ...
def actual_func(rank_t ...):
cdef:
rank_t val, thing
thing = get_thing(val)
| {{endif}} | ||
| if tiebreak == TIEBREAK_FIRST: | ||
| # need to use a stable sort here | ||
| _as = np.lexsort(keys=order) |
There was a problem hiding this comment.
Maybe reading wrong but this is duplicated in each conditional branch? Could clean in follow up if so
| for i in range(n): | ||
| sum_ranks += i + 1 | ||
| dups += 1 | ||
|
|
||
| {{if dtype == 'object'}} | ||
| val = util.get_value_at(sorted_data, i) |
There was a problem hiding this comment.
Is this line no longer required?
There was a problem hiding this comment.
AFAICT it hasnt been needed for a while. I recently removed most usages of util.get_value_at as unnecessary when we know i is an integer.
jbrockmendel
changed the title
| elif rank_t is int64_t: | ||
| nan_value = np.iinfo(np.int64).max | ||
|
|
||
| order = (values, mask) |
There was a problem hiding this comment.
can you add some comments here
There was a problem hiding this comment.
can you be more specific? there is a pretty nice comment up on L69
| if rank_t is object: | ||
| nan_value = Infinity() | ||
| elif rank_t is float64_t: | ||
| nan_value = np.inf |
|
thanks |
No description provided.