[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
| """ return the maximum size of elements in a 1-dim string array """ | ||
| cdef: | ||
| Py_ssize_t i, m = 0, l = 0, length = arr.shape[0] | ||
| pandas_string v | ||
|
|
||
| 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 gentle ping; got your OK last week |
|
think this was ok, can you rebase |
|
Rebased; circleCI fail is hypothesis timeout |
|
Rebased+green |
| @@ -587,7 +587,7 @@ def get_dispatch(dtypes): | |||
|
|
|||
| {{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?
| @@ -132,6 +132,7 @@ cdef inline void _sipround(uint64_t* v0, uint64_t* v1, | |||
| 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.
| @@ -107,7 +107,7 @@ def memory_usage_of_objects(object[:] arr): | |||
| # ---------------------------------------------------------------------- | |||
|
|
|||
|
|
|||
| 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! |
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)