Cythonize is_null_slice #55037
Cythonize is_null_slice #55037
Conversation
|
Is there a reason you want to Cythonize this? (impact on benchmarks?) This seems to be used only 6 times. |
|
Yea it also would get used in #54506 in the getitem / setitem methods of the BitmaskArray |
|
I won't claim that this has a significant impact on benchmarks. The main thing is that it shows less Python interaction in Cython annotate, so it causes less visual noise when searching through the annotate to find bottlenecks. If there's unease about this then no problem closing |
mroeschke
added
Missing-data
| except TypeError: | ||
| return False | ||
|
|
||
| if start == 0 and stop == PY_SSIZE_T_MAX and step == 1: |
There was a problem hiding this comment.
will this behave differently from the status quo on e.g. slice(0, None, 1)?
There was a problem hiding this comment.
No - PY_SSIZE_T_MAX is the largest unit used for addressing on the platform. CPython converts None to that internally, essentially letting you index as far out as your platform allows
There was a problem hiding this comment.
it's not the None here that im concerned with, its the 0 and 1. On main:
In [1]: import pandas.core.common as com
In [2]: obj = slice(0, None, 1)
In [3]: com.is_null_slice(obj)
Out[3]: False
There was a problem hiding this comment.
Hmm nice catch. That does return True here - are you thinking that is better or worse?
There was a problem hiding this comment.
i think the function is intended pretty specifically to catch obj[:], so would keep the status quo behavior
There was a problem hiding this comment.
OK cool. Thanks for catching that - I think can just close this for now then. Not a super high priority
This makes the diff in #54506 smaller