search

vallis / libstempo

libstempo — a Python wrapper for tempo2
MIT License
29 stars 34 forks source link

Change cpdef variables to cdef #54

Closed rossjjennings closed 1 year ago

rossjjennings commented 1 year ago

As I mentioned in #53, trying to install libstempo from source is giving me a bunch of Cython errors because of a change in Cython 3.0 that forbid cpdef for variables. Apparently there was no difference between cpdef and cdef in these contexts anyway. So this PR changes the offending cpdef lines to cdef.

AaronDJohnson commented 1 year ago

Hi @rossjjennings , will this also be compatible with Cython <3.0, or was there a change in the way that cdef works at the new version?

rossjjennings commented 1 year ago

It should also be compatible with the previous versions. Looking at the original Cython issue that gave rise to this here, my understanding is that there was never a difference between cdef and cpdef for variables, only for functions, and they decided to make it an error in 3.0 to discourage people from using cpdef variables.

rossjjennings commented 1 year ago

Also note that the current version of libstempo.pyx has this comment:

# TO DO: is cpdef required here?

Apparently it wasn't.