search

raysect / source

The main source repository for the Raysect project.
http://www.raysect.org
BSD 3-Clause "New" or "Revised" License
86 stars 23 forks source link

Cython v3 support #426

Open munechika-koyo opened 1 year ago

munechika-koyo commented 1 year ago

Hello,

I tried to resolve cython v3 incompatibilities by adding reversed binary arithmetic operators like __radd__, __rmul__, etc. Existing unit test has been passed, but I did not make sure that they cover all of methods. I would appreciate it if you would check & comment on this PR.

munechika-koyo commented 1 year ago

Thank you for confirming my PR! @vsnever

There is a draft PR with a proposal to add global constants (e.g. cdef const double CONST), https://github.com/cython/cython/pull/5242. I think that until this or similar functionality is implemented, DEF statements in Raysect or Cherab should remain.

Yes, I agree with you. We should follow this issue and check at which version DEF statement will be depricated.

Also, Cython 3 allows to get rid of that annoying deprecated NumPy-1.7 C-API warning. This can be done by defining the macro in setup.py:

I added some commits according to your recommendation. Please confirm it as well.

jacklovell commented 11 months ago

Cython currently generates tons of warnings about deprecated DEF statements. However, I think that so far there is no adequate replacement for them. Although many of these statements can be replaced with global cdef variables, this does not work for those that define the size of static arrays, like here:

It is possible to have compile time constant integers in Cython without DEF, by using an anonymous enum. Your example would become:

cdef enum:
    NN = 312 
    MM = 156 

 # The array for the state vector 
 cdef uint64_t mt[NN]

Is it clearer than a bunch of DEF statements? I'm not convinced. Is it nicer than being spammed with warnings about DEF being deprecated? Probably.

It would at least solve this specific case of defining the size of statically-sized arrays, as well as a bunch of other integer definitions in the code. The DEF INFINITY instances can be replaced by from math cimport INFINITY. There are also a lot of places in the code where DEF is used for floats which will need a different solution: none of them quite fit as math cimports.

vsnever commented 11 months ago

It would at least solve this specific case of defining the size of statically-sized arrays, as well as a bunch of other integer definitions in the code. The DEF INFINITY instances can be replaced by from math cimport INFINITY. There are also a lot of places in the code where DEF is used for floats which will need a different solution: none of them quite fit as math cimports.

It looks like the Cython developers have removed the deprecation warnings for DEF statements for now, https://github.com/cython/cython/issues/4310#issuecomment-1702887983. While switching to enum for integers is ok, I'd wait to see if the Cython developers come up with solution for compile-time constants that replaces DEF.