sagemath/sage

Meta-ticket: Cython 3.0.0 (released July 2023)

Closed this issue · 33 comments

This ticket is for experimenting with an update to Cython 3, which entered the beta stage on 2023-02-25 after several years in alpha. (No release schedule has been published.)

https://github.com/cython/cython/blob/master/CHANGES.rst

  • #34257 Add missing cdef method declarations (for Cython 3)
  • #26254 Use Cython directive binding=True to get signatures in help for cythonized built-in methods
  • #32832 cython: Eliminate use of custom patches
  • #30511 Type hints (__annotations__) and coercion/categories/Cython

See also:

Depends on #26254
Depends on #32832
Depends on #34257

CC: @kwankyu @williamstein @videlec @tscrim @tobiasdiez

Component: packages: standard

Keywords: upgrade

Branch/Commit: u/mkoeppe/meta_ticket__cython_3_0_0__unreleased_ @ 6c338f8

Issue created by migration from https://trac.sagemath.org/ticket/29863

slel commented

Changed keywords from none to upgrade

comment:3

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

Description changed:

--- 
+++ 
@@ -1,5 +1,9 @@
-This ticket is for experimenting with an update to Cython 3, currently at 3.0.0 alpha 5.
+This ticket is for experimenting with an update to Cython 3, which is at 3.0.0 alpha 10 as of 2022-07. No release schedule has been published (https://mail.python.org/pipermail/cython-devel/2019-November/thread.html)
 
 https://github.com/cython/cython/blob/master/CHANGES.rst
 
+- #26254 Use Cython directive binding=True to get signatures in help for cythonized built-in methods
+- #30511 Type hints (__annotations__) and coercion/categories/Cython
+- https://github.com/cython/cython/pull/4918 (Backport to 0.29.x: Add PEP420 namespace support)
 
+

Description changed:

--- 
+++ 
@@ -2,8 +2,8 @@
 
 https://github.com/cython/cython/blob/master/CHANGES.rst
 
-- #26254 Use Cython directive binding=True to get signatures in help for cythonized built-in methods
-- #30511 Type hints (__annotations__) and coercion/categories/Cython
+- #26254 Use Cython directive `binding=True` to get signatures in help for cythonized built-in methods
+- #30511 Type hints (`__annotations__`) and coercion/categories/Cython
 - https://github.com/cython/cython/pull/4918 (Backport to 0.29.x: Add PEP420 namespace support)
 
 

Description changed:

--- 
+++ 
@@ -1,4 +1,7 @@
-This ticket is for experimenting with an update to Cython 3, which is at 3.0.0 alpha 10 as of 2022-07. No release schedule has been published (https://mail.python.org/pipermail/cython-devel/2019-November/thread.html)
+This ticket is for experimenting with an update to Cython 3, which is at 3.0.0 alpha 10 as of 2022-07. No release schedule has been published.
+- https://mail.python.org/pipermail/cython-devel/2019-November/thread.html (2019 - no response)
+- https://groups.google.com/g/cython-users/c/WTHmGJ44aB0 (2022-01 "no concrete time plan")
+- https://github.com/cython/cython/issues/4022
 
 https://github.com/cython/cython/blob/master/CHANGES.rst
 

Description changed:

--- 
+++ 
@@ -1,7 +1,7 @@
 This ticket is for experimenting with an update to Cython 3, which is at 3.0.0 alpha 10 as of 2022-07. No release schedule has been published.
 - https://mail.python.org/pipermail/cython-devel/2019-November/thread.html (2019 - no response)
 - https://groups.google.com/g/cython-users/c/WTHmGJ44aB0 (2022-01 "no concrete time plan")
-- https://github.com/cython/cython/issues/4022
+- https://github.com/cython/cython/issues/4022 (still no road map)
 
 https://github.com/cython/cython/blob/master/CHANGES.rst
 

Description changed:

--- 
+++ 
@@ -6,7 +6,10 @@
 https://github.com/cython/cython/blob/master/CHANGES.rst
 
 - #26254 Use Cython directive `binding=True` to get signatures in help for cythonized built-in methods
+- #32832 `cython`: Eliminate use of custom patches
 - #30511 Type hints (`__annotations__`) and coercion/categories/Cython
+
+See also:
 - https://github.com/cython/cython/pull/4918 (Backport to 0.29.x: Add PEP420 namespace support)
 
 

Dependencies: #26254, #32832

Commit: 02ce997

comment:13

pplpy build fails:

    tree = Parsing.p_module(s, pxd, full_module_name)

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  from ppl_decl cimport *
  ^
  ------------------------------------------------------------

  ppl/bit_arrays.pxd:1:0: 'ppl_decl.pxd' not found

  Error compiling Cython file:

Last 10 new commits:

bea228bTry with localized import
89b8081Inline all other imports from expression
9bbf574Fix a few tests
803804fand one more test
a23ffe9Tiny edits
570058cMerge remote-tracking branch 'origin/u/mkoeppe/update_cython_to_0_29_29' into public/26254
02d6282Merge remote-tracking branch 'origin/develop' into public/26254
bca67b0Merge #26254
f537175build/pkgs/cython: Use 3.0.0a10
02ce997build/pkgs/cython/patches/trashcan.patch: Remove
comment:14
[sagelib-9.7.beta6]   Error compiling Cython file:
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6]   ...
[sagelib-9.7.beta6]               sage: p = next_prime(2^32)
[sagelib-9.7.beta6]               sage: GF(p)(int(p+1))
[sagelib-9.7.beta6]               1
[sagelib-9.7.beta6]           """
[sagelib-9.7.beta6]           mpz_set_si(self.value, value)
[sagelib-9.7.beta6]           mpz_mod(self.value, self.value, self.__modulus.sageInteger.value)
[sagelib-9.7.beta6]                                                                    ^
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]   sage/rings/finite_rings/integer_mod.pyx:1986:66: Cannot convert Python object to '__mpz_struct *'
comment:15
[sagelib-9.7.beta6]   Error compiling Cython file:
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6]   ...
[sagelib-9.7.beta6]           """
[sagelib-9.7.beta6]           Lfunction.__init__(self, name, what_type_L, dirichlet_coefficient,  period, Q, OMEGA, gamma,lambd, pole,residue)
[sagelib-9.7.beta6]           self._repr += " with integer Dirichlet coefficients"
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]       # override
[sagelib-9.7.beta6]       cdef void __init_fun(self, char *NAME, int what_type, dirichlet_coeff, long long Period, double q,  c_Complex w, int A, double *g, c_Complex *l, int n_poles, c_Complex *p, c_Complex *r):
[sagelib-9.7.beta6]           ^
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]   sage/libs/lcalc/lcalc_Lfunction.pyx:500:9: C method '_Lfunction_I__init_fun' not previously declared in definition part of extension type 'Lfunction_I'
comment:16
[sagelib-9.7.beta6]   Error compiling Cython file:
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6]   ...
[sagelib-9.7.beta6]           """
[sagelib-9.7.beta6]           return Matroid._repr_(self) + " with " + str(self.bases_count()) + " bases"
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]       # support for parent BasisExchangeMatroid
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]       cdef bint __is_exchange_pair(self, long x, long y) except -1:      # test if current_basis-x + y is a basis
[sagelib-9.7.beta6]           ^
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]   sage/matroids/basis_matroid.pyx:268:9: C method '_BasisMatroid__is_exchange_pair' not previously declared in definition part of extension type 'BasisMatroid'

[sagelib-9.7.beta6]   Error compiling Cython file:
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6]   ...
[sagelib-9.7.beta6]                   b = frozenset(B)
[sagelib-9.7.beta6]                   if len(b) != self._matroid_rank:
[sagelib-9.7.beta6]                       raise ValueError("basis has wrong cardinality.")
[sagelib-9.7.beta6]                   if not b.issubset(self._groundset):
[sagelib-9.7.beta6]                       raise ValueError("basis is not a subset of the groundset")
[sagelib-9.7.beta6]                   self.__pack(self._b, b)
[sagelib-9.7.beta6]                                  ^
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]   sage/matroids/basis_matroid.pyx:224:32: Cannot convert 'bitset_t' to Python object
[sagelib-9.7.beta6] 
comment:17
[sagelib-9.7.beta6]   Error compiling Cython file:
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6]   ...
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]       AUTHOR:
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]       - Xavier Caruso (2013-03)
[sagelib-9.7.beta6]       """
[sagelib-9.7.beta6]       cdef int __normalize(self) except -1:
[sagelib-9.7.beta6]           ^
[sagelib-9.7.beta6]   ------------------------------------------------------------
[sagelib-9.7.beta6] 
[sagelib-9.7.beta6]   sage/rings/polynomial/polynomial_element.pyx:11912:9: C method '_Polynomial_generic_dense_inexact__normalize' not previously declared in definition part of extension type 'Polynomial_generic_dense_inexact'

Branch pushed to git repo; I updated commit sha1. New commits:

fd2e122src/sage/libs/lcalc/lcalc_Lfunction.pxd: Add missing cdef method declarations

Changed commit from 02ce997 to fd2e122

Changed commit from fd2e122 to 4e14632

Branch pushed to git repo; I updated commit sha1. New commits:

4e14632src/sage/matroids/{basis,linear}_matroid.pxd: Add missing cdef method declarations

Changed commit from 4e14632 to 46af90e

Branch pushed to git repo; I updated commit sha1. New commits:

46af90esrc/sage/rings/polynomial/polynomial_element.pxd: Add missing cdef method declaration
comment:21

Help wanted with Cannot convert 'bitset_t' to Python object and Cannot convert Python object to '__mpz_struct *'

Description changed:

--- 
+++ 
@@ -10,6 +10,6 @@
 - #30511 Type hints (`__annotations__`) and coercion/categories/Cython
 
 See also:
-- https://github.com/cython/cython/pull/4918 (Backport to 0.29.x: Add PEP420 namespace support)
+- #34221 / https://github.com/cython/cython/pull/4918 (Backport to 0.29.x: Add PEP420 namespace support)
 
 

Changed dependencies from #26254, #32832 to #26254, #32832, #34257

Description changed:

--- 
+++ 
@@ -5,6 +5,7 @@
 
 https://github.com/cython/cython/blob/master/CHANGES.rst
 
+- #34257 Add missing `cdef` method declarations (for Cython 3)
 - #26254 Use Cython directive `binding=True` to get signatures in help for cythonized built-in methods
 - #32832 `cython`: Eliminate use of custom patches
 - #30511 Type hints (`__annotations__`) and coercion/categories/Cython

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1f9ee80src/sage/libs/lcalc/lcalc_Lfunction.pxd: Add missing cdef method declarations
034094csrc/sage/matroids/{basis,linear}_matroid.pxd: Add missing cdef method declarations
9d89bdfsrc/sage/rings/polynomial/polynomial_element.pxd: Add missing cdef method declaration
727d101Merge #34257
6c338f8build/pkgs/pplpy/patches: Add https://gitlab.com/videlec/pplpy/-/merge_requests/20

Changed commit from 46af90e to 6c338f8

comment:25

Replying to @mkoeppe:

pplpy build fails:

    tree = Parsing.p_module(s, pxd, full_module_name)

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  from ppl_decl cimport *
  ^
  ------------------------------------------------------------

  ppl/bit_arrays.pxd:1:0: 'ppl_decl.pxd' not found

  Error compiling Cython file:

PR: https://gitlab.com/videlec/pplpy/-/merge_requests/20

Now that 3.0.0 is released, could we have this rebased and in a PR, please?

Cf: https://github.com/tornaria/sage/tree/cython3-legacy, I've got as far as being able to cythonize everything, but some C files still fail to compile.

First five commits are git cherry-pick f36275a7869 36ec4930832 1f9ee80cded 034094c849e 9d89bdf776 from the sagetrac mirror on top of 10.1.beta7.

Note that cython/cython#5552 means cythonize fails if python package lxml is available. Can use SAGE_DEBUG=no to avoid the issue which only happens with debug enabled.

Also: void-linux/void-packages#45086 includes patches for cysignals, cypari2, fpylll and pplpy that might be useful.

After adding a commit fixing inheritance in lcalc_Lfunction (see 10.1.beta7...tornaria:sage:d0ad68ed9d4) I've got the whole of sagelib to compile.

Now it fails at import sage.structure.element as follows:

$ PYTHONPATH=pkgs/sagemath-standard/build/lib.linux-x86_64-cpython-311 python
Python 3.11.4 (main, Jun  8 2023, 02:02:15) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sage.structure.element
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../pkgs/sagemath-standard/build/lib.linux-x86_64-cpython-311/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element (build/cythonized/sage/structure/element.c:44218)
TypeError: PyMethodDescr_CallSelf requires a method without arguments

Please note: I'll probably keep rebasing and/or changing commits in my branch https://github.com/tornaria/sage/tree/cython3-legacy so don't take them as public commits unless you are willing to rebase.

Branch pushed to git repo; I updated commit sha1. New commits:
fd2e122 src/sage/libs/lcalc/lcalc_Lfunction.pxd: Add missing cdef method declarations

This is incorrect and it stems from a fundamental change in cython 3 that will probably bite us all over the place (and this one cannot be switched off afaict).

I better link to the relevant section of the migration guide: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#class-private-name-mangling

This means sagelib has been (mis)using __name members (methods and attributes) that (a) in python are class-private via name mangling (b) in cython before 3.0.0 they are inherited due to mangling not implemented (c) in cython 3.0.0 they are class-private via the exact same name mangling as in python.

So the way to fix all these "missing cdef ..." errors is not by adding the missing methods but renaming the members to use a single underscore which is the python way to declare a "private" member that is still inherited by subclasses (think "protected" in c++).