[CLN] More cython cleanups, with bonus type annotations #22283
Conversation
|
Out of curiosity how are you verifying that the |
Just text search within the file. Since the relevant functions aren't exposed in a .pxd file, there is no risk of them being called from a different cython file. |
Codecov Report
@@ Coverage Diff @@
## master #22283 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 169 169
Lines 50782 50782
=======================================
Hits 46742 46742
Misses 4040 4040
Continue to review full report at Codecov.
|
gfyoung
added
Internals
|
@jreback would this be easier if I separated the no-profile comments into an isolated PR? |
|
no just haven’t had a chance to look yet |
jbrockmendel
changed the title
| for i in range(length): | ||
| v = arr[i] | ||
| if PyString_Check(v): | ||
| if isinstance(v, str): |
There was a problem hiding this comment.
do these make any perf diffs?
There was a problem hiding this comment.
cython makes this substitution on its own
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
looks ok, can you rebase
|
@jreback gentle ping; got your OK last week |
|
think this was ok, can you rebase |
|
Rebased; circleCI fail is hypothesis timeout |
|
Rebased+green |
| {{for name, c_type, dtype in get_dispatch(dtypes)}} | ||
|
|
||
| cpdef ensure_{{name}}(object arr, copy=True): | ||
| def ensure_{{name}}(object arr, copy=True): |
There was a problem hiding this comment.
are these for sure not called in cython code?
| v2[0] = _rotl(v2[0], 32) | ||
|
|
||
|
|
||
| # TODO: This appears unused; remove? |
There was a problem hiding this comment.
might be unused - it was a part of the hashing at one point iirc
There was a problem hiding this comment.
OK, will remove in next pass.
|
|
||
|
|
||
| cpdef bint is_scalar(object val): | ||
| def is_scalar(val: object) -> bint: |
There was a problem hiding this comment.
this not ever called in cython?
There was a problem hiding this comment.
Yep, easy to confirm via grep
|
thanks! |
4 participants
A few places where string formatting is modernized
Removed remaining
# cython: profile=FalsecommentsThe main (and possible controversial) thing done here is changing some functions to use type-hint syntax. The affected functions are all currently
cpdef, but are never used in cython, so should only bedef. But cython's syntax does not allow for specifying a return type indeffunctions, so this is the cleanest way to remove unnecessarycpdefwithout losing typing. (some discussion of this occurred in #22180)