Remove inline declarations in pxd files #17277
Conversation
At the moment they dont do anything but cause lots of warnings during cythonizing. Eventually they will raise errors. cython/cython#1706 (comment)
gfyoung
added
Clean
Internals
Codecov Report
@@ Coverage Diff @@
## master #17277 +/- ##
==========================================
- Coverage 91% 90.99% -0.02%
==========================================
Files 162 162
Lines 49547 49547
==========================================
- Hits 45091 45083 -8
- Misses 4456 4464 +8
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17277 +/- ##
==========================================
- Coverage 91% 90.99% -0.02%
==========================================
Files 162 162
Lines 49547 49547
==========================================
- Hits 45091 45083 -8
- Misses 4456 4464 +8
Continue to review full report at Codecov.
|
|
this would need a performance check. older versions of cython may respect this directive. |
| inline khint_t kh_put_pymap(kh_pymap_t*, PyObject*, int*) | ||
| inline void kh_del_pymap(kh_pymap_t*, khint_t) | ||
| kh_pymap_t* kh_init_pymap() | ||
| void kh_destroy_pymap(kh_pymap_t*) |
There was a problem hiding this comment.
But they still trigger the warning when building. So apparently that does not matter
There was a problem hiding this comment.
Based on the comment in the cython issues, I understand that those will not be inlined anyhow when used in another cython module (cython/cython#1706 (comment))
| @@ -38,7 +38,7 @@ cdef class MultiIndexHashTable(HashTable): | |||
|
|
|||
| cpdef get_item(self, object val) | |||
| cpdef set_item(self, object key, Py_ssize_t val) | |||
| cdef inline void _check_for_collision(self, Py_ssize_t loc, object label) | |||
There was a problem hiding this comment.
this prob should be implemented in the .pxd file; its called a lot
There was a problem hiding this comment.
If this is just a matter of cut/pasting from the pxi file, you got it.
There was a problem hiding this comment.
Is it correct that _check_for_collisions is only used in hashtable.pyx itself? (or in the included template files)
If so, I think we can just remove it here from the pxd file altogether (it is already defined as inline in hashtable itself)
|
The comment above only seems to apply to cython defintiion, NOT c definitions. |
OK, then this is moving outside my competence zone. We can wait until it starts raising errors and fix it then. |
|
ok, as I said its new in cython 0.26 if you want to specifically address some things great. but will need performance checking. |
At the moment (I think since .26, not sure) they don't do anything but cause lots of warnings during cythonizing. Eventually they will raise errors.
cython/cython#1706 (comment)
git diff upstream/master -u -- "*.py" | flake8 --diff