Removed PyString refs from extension modules #28322
Conversation
| return PyString_FromString(data); | ||
| #endif | ||
| } | ||
| """ | ||
| object char_to_string(const char* data) |
There was a problem hiding this comment.
shouldnt L14 be removed too?
There was a problem hiding this comment.
It's actually used here:
pandas/pandas/_libs/tslibs/period.pyx
Line 1267 in 2d65e38
So maybe but would require a more comprehensive refactor
There was a problem hiding this comment.
I think that just becomes result = PyUnicode_FromString(formatted)
There was a problem hiding this comment.
Cool - I'll give it a look. Seems like this caused a somewhat surprising failure so that might solve
There was a problem hiding this comment.
Hmm so didn't realize the cdef of the tripled quoted string was actually using that as a macro. Was thinking of cimporting PyUnicode_FromString directly but looks like that isn't available until Cython 3.0.0
For now going to revert to macro
| @@ -2,23 +2,13 @@ | |||
| from cpython cimport PyTypeObject | |||
|
|
|||
| cdef extern from *: | |||
| """ | |||
| PyObject* char_to_string(const char* data) { | |||
| #if PY_VERSION_HEX >= 0x03000000 | |||
There was a problem hiding this comment.
I think there's one more PY_VERSION_HEX check in compat_helper that could go
There was a problem hiding this comment.
Yea I was looking at that but I'm planning on the compat stuff in a separate PR
|
thanks @WillAyd |
* Removed PyString refs from extension modules * Reverted macro
* Removed PyString refs from extension modules * Reverted macro
We have some old PyString references hanging around in our extension modules. These are a relic of the old Py2 string handling and will be removed in Python 4 I think, so figured worth modernizing
https://docs.python.org/3/howto/cporting.html