Update Cython to 3.0.2 #36110

infmagic2047 · 2023-08-21T03:17:54Z

This PR builds on #36109 and contains the rest of changes needed to update Cython to 3.0.0. The actual changes needed for building Sage are a632238 for the code, 67ad2e4 for the build system, and 7195b8f for the doctests. The rest of commits are either from #36109 or for patching dependencies.

Closes #29863.

Ideally we should wait until numpy and scipy are upgraded to 1.25 and 1.11 respectively before merging this, so we can drop the patches for them.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.

⌛ Dependencies

Depends on Prepare for updating Cython to 3.0.0 #36109
Depends on Upgrades: scipy 1.11 and dependencies #36112 (merged here)

tornaria · 2023-08-30T01:23:42Z

Can you please rebase on top of current #36109 (i.e. 4b85613)?

tornaria · 2023-08-30T03:10:21Z

Thanks. On a first attempt everything is looking great.

I had this:

sage -t --random-seed=16985807869938045917183290929540248820 src/sage/graphs/connectivity.pyx
**********************************************************************
File "src/sage/graphs/connectivity.pyx", line 2817, in sage.graphs.connectivity._Component.__init__
Failed example:
    cython(os.linesep.join(cython_code))                                  # needs sage.misc.cython
Expected:
    Polygon: 2 3 4
Got:
    warning: /usr/lib/python3.11/site-packages/cysignals/signals.pxd:57:56: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.
    Polygon: 2 3 4
**********************************************************************
File "src/sage/graphs/connectivity.pyx", line 2864, in sage.graphs.connectivity._Component.__str__
Failed example:
    cython(os.linesep.join(cython_code))                                  # needs sage.misc.cython
Expected:
    Polygon: 2 3 4
Got:
    warning: /usr/lib/python3.11/site-packages/cysignals/signals.pxd:57:56: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.
    Polygon: 2 3 4
**********************************************************************
2 items had failures:
   1 of   3 in sage.graphs.connectivity._Component.__init__
   1 of   3 in sage.graphs.connectivity._Component.__str__
    [523 tests, 2 failures, 9.23 s]
----------------------------------------------------------------------
sage -t --random-seed=16985807869938045917183290929540248820 src/sage/graphs/connectivity.pyx  # 2 doctests failed
----------------------------------------------------------------------

Are you getting this failure?

This is just a warning coming from cysignals, which seems it could be fixed by

--- a/src/cysignals/signals.pxd.in
+++ b/src/cysignals/signals.pxd.in
@@ -54,7 +54,7 @@ cdef extern from "macros.h" nogil:
 # can be used to make Cython check whether there is a pending exception
 # (PyErr_Occurred() is non-NULL). To Cython, it will look like
 # cython_check_exception() actually raised the exception.
-cdef inline void cython_check_exception() nogil except *:
+cdef inline void cython_check_exception() except * nogil:
     pass

tornaria · 2023-08-30T03:20:38Z

src/sage/arith/srange.pyx

@@ -84,7 +84,7 @@ def xsrange(start, end=None, step=1, universe=None, *, coerce=True, bint include
    EXAMPLES::

        sage: xsrange(10)
-        <generator object at 0x...>
+        <_cython_3_0_0.generator object at 0x...>


Suggested change

<_cython_3_0_0.generator object at 0x...>

<_cython_3_....generator object at 0x...>

or maybe just

Suggested change

<_cython_3_0_0.generator object at 0x...>

<...generator object at 0x...>

since the original one is already failing with cython 3.0.2 (which I'm testing with right now).

tornaria · 2023-08-30T03:28:44Z

Except the cysignals warning, everything passes (I didn't run --long yet):

with cython 3.0.0
with cython 3.0.2 (except the issue with generators that I mentioned above)

This is looking very promising. More testing necessary. I hope #36109 is merged soon so it becomes much easier to play with this PR.

tornaria · 2023-08-30T03:29:42Z

BTW, I added the patch to cysignals that I suggested above; then the related doctest failure is gone.

tornaria · 2023-08-30T23:56:31Z

I ran all doctests (normal and --long) on

x86_64 glibc
x86_64 musl
i686 glibc

Everything passes.

Not a serious benchmark yet, but it feels like doctests are taking a bit longer (maybe ~10%, but I'm not sure as the computer might have been loaded)... I will test more carefully later.

infmagic2047 · 2023-08-31T18:02:02Z

About the cysignals failure, I got the same on my system, but I did not include the patch here since I consider it more appropriate to fix cysignals directly and publish a new version there.

kwankyu · 2023-09-01T11:46:38Z

src/sage_setup/cython_options.py

@@ -10,13 +10,17 @@ def compiler_directives(profile: bool):
        auto_pickle=False,
        # Do not create __test__ dictionary automatically from docstrings
        autotestdict=False,
+        binding=False,
+        c_api_binop_methods=True,


Why binding=False?

binding=True is a present that we were expecting from cython3 for a long time. #26254

I think the goal of this PR is to be able to build sagemath with cython 3. It seems enabling options to make cython 3 keep legacy behaviour is the minimal working step we can make. After sagemath is built with cython 3, it can be explored how to switch to the new behaviour. Same for legacy_implicit_noexcept, which can be removed after all the implicit noexcept are made explicit, etc.

Fair enough.

tornaria · 2023-09-02T21:28:35Z

Please rebase to 10.2.beta1. Now that #36109 is merged, I think this is ready for review, in fact it works very well for me, I am almost ready to give positive review. Caveats:

I'm only building sagelib, and I haven't tested the spkg updates, etc.
cython should be updated to 3.0.2 instead of 3.0.0
maybe cysignals should be patched... I agree it's better to fix it upstream but until there is a new release of cysignals with this fixed, this is a simple patch (and it will be easy to catch and remove when this is fixed upstream).

For some reason @infmagic2047 PR is not getting tested by CI: "This workflow requires approval from a maintainer." maybe some maintainer can approve the workflow (and add them to whatever group is necessary so they can run workflows without approval). I don't have workflow approval powers myself.

tornaria · 2023-09-02T21:30:09Z

For cysignals fix I mentioned, here's the patch we are using: https://github.com/void-linux/void-packages/blob/master/srcpkgs/python3-cysignals/patches/cython3-fix-warning.patch

infmagic2047 · 2023-09-04T07:25:11Z

I rebased on develop and added the cysignals patch.

tornaria · 2023-09-05T01:27:12Z

LGTM. In void-linux/void-packages#45887 I build and doctest sagemath with cython 3 on the following architectures:

x86_64 with glibc
x86_64 with musl libc
i686 with glibc

orlitzky · 2023-09-20T00:43:02Z

So far, I have not done anything on that front. I do not have as much time as I used to. But I would appreciate some roll out of release from the various dependencies rather than patch to them. cysignals, cypari, does memory_allocator, fpylll need anything done?

Tobias says no but I've only tried it with the patched versions so far.

kiwifb · 2023-09-20T00:46:52Z

So far, I have not done anything on that front. I do not have as much time as I used to. But I would appreciate some roll out of release from the various dependencies rather than patch to them. cysignals, cypari, does memory_allocator, fpylll need anything done?

Tobias says no but I've only tried it with the patched versions so far.

Well, I see patches for all of them (except memory_allocator) plus pplpy that I had forgotten in the PR. Frankly all those should have new releases. Otherwise, all the packages have to patched individually. I guess I can do some release of all of them in sage-on-gentoo to test.

kiwifb · 2023-09-20T22:24:58Z

So, I tried to insert all that stuff in my sage-on-gentoo development tree and stable cython (3.0.0) and pythran (0.13.1). I got the following failures when cythonizing sage

Error compiling Cython file:
------------------------------------------------------------
...
"""
^
------------------------------------------------------------

sage/algebras/finite_dimensional_algebras/finite_dimensional_algebra_element.pyx:1:0: Compiler crash in DebugTransform


Compiler crash traceback from this point on:
  File "Cython/Compiler/Visitor.py", line 182, in Cython.Compiler.Visitor.TreeVisitor._visit
  File "/usr/lib/python3.11/site-packages/Cython/Compiler/ParseTreeTransforms.py", line 4033, in visit_ModuleNode
    self.visit_FuncDefNode(nested_funcdef)
  File "/usr/lib/python3.11/site-packages/Cython/Compiler/ParseTreeTransforms.py", line 4093, in visit_FuncDefNode
    self.tb.start(arg.name)
  File "/usr/lib/python3.11/site-packages/Cython/Debugger/DebugWriter.py", line 42, in start
    self.tb.start(name, attrs or {})
  File "src/lxml/saxparser.pxi", line 841, in lxml.etree.TreeBuilder.start
  File "src/lxml/saxparser.pxi", line 769, in lxml.etree.TreeBuilder._handleSaxStart
  File "src/lxml/apihelpers.pxi", line 179, in lxml.etree._makeSubElement
  File "src/lxml/apihelpers.pxi", line 1754, in lxml.etree._tagValidOrRaise
ValueError: Invalid tag name '.0'
Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1330, in cythonize_one_helper
    return cythonize_one(*m)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1306, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: sage/algebras/finite_dimensional_algebras/finite_dimensional_algebra_element.pyx

and similar messages for quite a lot of files. I'd say I missed a patch or a dependency needs to be raised.

tornaria · 2023-09-20T23:12:05Z

lxml

Use cython/cython#5690 or set SAGE_DEBUG=no.

kiwifb · 2023-09-20T23:25:51Z

lxml

Use cython/cython#5690 or set SAGE_DEBUG=no.

OK, SAGE_DEBUG=no let me go past that to compile. Let's see what happens next.

kiwifb · 2023-09-21T01:36:37Z

Well, sage-on-gentoo builds and doctest fine with the patches here for the dependencies and sage itself. I had to export SAGE_DEBUG=no as suggested by @tornaria but it otherwise did fine. I ended going for cython 3.0.2 and pythran 0.14.0. I will look to see if some of the optional dependencies I have in my tree also build.

kiwifb · 2023-09-21T01:58:08Z

There seems to be an issue with sagemath-meataxe

Error compiling Cython file:
------------------------------------------------------------
...
from sage.matrix.matrix_space import MatrixSpace
from sage.misc.randstate import current_randstate
from sage.misc.randstate cimport randstate
from sage.structure.element cimport Element, Matrix
from sage.structure.richcmp import rich_to_bool
from .args cimport MatrixArgs_init
^
------------------------------------------------------------

sage/matrix/matrix_gfpn_dense.pyx:58:0: 'sage/matrix/args.pxd' not found

Error compiling Cython file:
------------------------------------------------------------
...
            Vector space of degree 2 and dimension 2 over Finite Field in y of size 5^3
            Basis matrix:
            [1 0]
            [0 1]
        """
        ma = MatrixArgs_init(parent, entries)
             ^
------------------------------------------------------------

sage/matrix/matrix_gfpn_dense.pyx:428:13: undeclared name not builtin: MatrixArgs_init
Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1330, in cythonize_one_helper
    return cythonize_one(*m)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1306, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: sage/matrix/matrix_gfpn_dense.pyx

Feels like something waiting to happen.

Edit: Replacing the offending line with from sage.matrix.args cimport MatrixArgs_init is enough to fix things.

mkoeppe · 2023-09-21T02:04:24Z

I have some fixes for such relative cimports on #35095 in dfceb98

kiwifb · 2023-09-21T02:07:04Z

I have some fixes for such relative cimports on #35095 in dfceb98

4253 files changed, who do you expect to review that? It cannot be reviewed by a human apart from "does it build on my systems."

mkoeppe · 2023-09-21T02:10:09Z

The cited commit only changes 314 files...

kiwifb · 2023-09-21T02:13:48Z

The cited commit only changes 314 files...

Indeed, and my guilty file is in there. Well done. But that's still "only 314".

mkoeppe · 2023-09-21T02:14:52Z

4253 files changed, who do you expect to review that? It cannot be reviewed by a human apart from "does it build on my systems."

#35095 is not ready for review (and indeed not reviewable); I am upstreaming its changes in smaller, reviewable chunks such as #36272 and #36271

kiwifb · 2023-09-21T02:25:09Z

The good news is that out of the optional packages I ship, this is the only that had an issue.

mkoeppe · 2023-09-21T02:37:06Z

Then let's push this one change here on the branch.
The 313 other changes are towards #36228

github-actions · 2023-09-21T06:00:27Z

Documentation preview for this PR (built with commit cb78fd9; changes) is ready! 🎉

mkoeppe · 2023-09-24T22:02:23Z

Does anyone have more concerns here, or can we merge it?

Various package upgrade PRs are waiting for it

tornaria · 2023-09-24T22:19:08Z

LGTM

dimpase · 2023-09-25T15:36:00Z

do we want to update on the latest develop branch?

NumPy 1.26 provides Python 3.12 support and Cython 3 compatibility. 1.26.0 was released 2023-09-16. As NumPy has changed its build system from setuptools-pinned-to-an- ancient-version to meson_python, we get rid of our ancient version of `setuptools`. The SPKGs `setuptools` and `setuptools_wheel` now ship the same (current) version. - [x] Check portability run: https://github.com/mkoeppe/sage/actions/runs/5959239800 - [ ] Check SAGE_FAT_BINARY   Resolves sagemath#34816  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies - Depends on sagemath#36112 (for the `meson_python` upgrades, merged here) - Depends on sagemath#35404 (merged here as part of above) - Depends on sagemath#35810 (merged here as part of above) - Depends on sagemath#36110 (merged here) - Depends on sagemath#36263 (merged here) - Depends on sagemath#36238 (merged here) - Depends on sagemath#36255 (merged here)   URL: sagemath#36123 Reported by: Matthias Köppe Reviewer(s):

NumPy 1.26 provides Python 3.12 support and Cython 3 compatibility. 1.26.0 was released 2023-09-16. As NumPy has changed its build system from setuptools-pinned-to-an- ancient-version to meson_python, we get rid of our ancient version of `setuptools`. The SPKGs `setuptools` and `setuptools_wheel` now ship the same (current) version. - [x] Check portability run: https://github.com/mkoeppe/sage/actions/runs/5959239800 - [ ] Check SAGE_FAT_BINARY   Resolves #34816  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies - Depends on #36112 (for the `meson_python` upgrades, merged here) - Depends on #35404 (merged here as part of above) - Depends on #35810 (merged here as part of above) - Depends on #36110 (merged here) - Depends on #36263 (merged here) - Depends on #36238 (merged here) - Depends on #36255 (merged here)   URL: #36123 Reported by: Matthias Köppe Reviewer(s):

infmagic2047 mentioned this pull request Aug 21, 2023

Update Cython to 3.0.0 #36054

Closed

3 tasks

mkoeppe added this to the sage-10.2 milestone Aug 21, 2023

mkoeppe added p: critical / 2 c: packages: standard labels Aug 21, 2023

This was referenced Aug 22, 2023

Prepare for updating Cython to 3.0.0 #36109

Merged

python3-Cython: update to 3.0.1. void-linux/void-packages#45086

Closed

infmagic2047 force-pushed the use-cython-3 branch from 1af038f to da994e1 Compare August 30, 2023 03:00

tornaria reviewed Aug 30, 2023

View reviewed changes

tornaria mentioned this pull request Aug 30, 2023

Sagemath cython3 ahesford/void-packages#2

Closed

kwankyu reviewed Sep 1, 2023

View reviewed changes

tornaria mentioned this pull request Sep 3, 2023

sagemath: use cython 3 void-linux/void-packages#45887

Closed

infmagic2047 added 7 commits September 4, 2023 15:02

fix PY_NEW signature

326f36e

update doctests for cython 3

cba7b31

update cython to 3.0.2

703ec1b

use cython legacy directives for sage

ad75f66

add cython3 patches for cysignals, cypari2, fpylll, and pplpy

0044e67

add cython3 patch for scipy

bbe8134

add cython3 patch for numpy

5439175

infmagic2047 force-pushed the use-cython-3 branch from 6f26c85 to 5439175 Compare September 4, 2023 07:18

tornaria requested a review from mkoeppe September 5, 2023 01:29

src/sage/matrix/matrix_gfpn_dense.pyx: Replace relative cimport

cb78fd9

tornaria approved these changes Sep 24, 2023

View reviewed changes

orlitzky mentioned this pull request Sep 25, 2023

Yet more more spkgs confs #36332

Merged

mkoeppe added s: positive review and removed s: needs review labels Sep 26, 2023

vbraun merged commit 05d7d1d into sagemath:develop Sep 27, 2023
19 of 37 checks passed

github-actions bot removed the s: positive review label Sep 27, 2023

dimpase mentioned this pull request Sep 28, 2023

In doctesting, ignore two ld warnings from recent OS X: #36337

Closed

5 tasks

tobiasdiez mentioned this pull request Oct 7, 2023

Fix conda install instructions after setuptools update #36411

Merged

5 tasks

tornaria mentioned this pull request Apr 13, 2024

remove a stray leftover in in sa2si_ZZmod() #37792

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Cython to 3.0.2 #36110

Update Cython to 3.0.2 #36110

infmagic2047 commented Aug 21, 2023 •

edited by mkoeppe

Loading

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023

tornaria Aug 30, 2023

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023 •

edited

Loading

infmagic2047 commented Aug 31, 2023

kwankyu Sep 1, 2023

tornaria Sep 2, 2023

kwankyu Sep 4, 2023

tornaria commented Sep 2, 2023

tornaria commented Sep 2, 2023

infmagic2047 commented Sep 4, 2023

tornaria commented Sep 5, 2023

orlitzky commented Sep 20, 2023

kiwifb commented Sep 20, 2023

kiwifb commented Sep 20, 2023

tornaria commented Sep 20, 2023

kiwifb commented Sep 20, 2023

kiwifb commented Sep 21, 2023

kiwifb commented Sep 21, 2023 •

edited

Loading

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

github-actions bot commented Sep 21, 2023

mkoeppe commented Sep 24, 2023

tornaria commented Sep 24, 2023

dimpase commented Sep 25, 2023

	<_cython_3_0_0.generator object at 0x...>
	<_cython_3_....generator object at 0x...>

	<_cython_3_0_0.generator object at 0x...>
	<...generator object at 0x...>

Update Cython to 3.0.2 #36110

Update Cython to 3.0.2 #36110

Conversation

infmagic2047 commented Aug 21, 2023 • edited by mkoeppe Loading

📝 Checklist

⌛ Dependencies

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023

tornaria Aug 30, 2023

Choose a reason for hiding this comment

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023

tornaria commented Aug 30, 2023 • edited Loading

infmagic2047 commented Aug 31, 2023

kwankyu Sep 1, 2023

Choose a reason for hiding this comment

tornaria Sep 2, 2023

Choose a reason for hiding this comment

kwankyu Sep 4, 2023

Choose a reason for hiding this comment

tornaria commented Sep 2, 2023

tornaria commented Sep 2, 2023

infmagic2047 commented Sep 4, 2023

tornaria commented Sep 5, 2023

orlitzky commented Sep 20, 2023

kiwifb commented Sep 20, 2023

kiwifb commented Sep 20, 2023

tornaria commented Sep 20, 2023

kiwifb commented Sep 20, 2023

kiwifb commented Sep 21, 2023

kiwifb commented Sep 21, 2023 • edited Loading

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

kiwifb commented Sep 21, 2023

mkoeppe commented Sep 21, 2023

github-actions bot commented Sep 21, 2023

mkoeppe commented Sep 24, 2023

tornaria commented Sep 24, 2023

dimpase commented Sep 25, 2023

infmagic2047 commented Aug 21, 2023 •

edited by mkoeppe

Loading

tornaria commented Aug 30, 2023 •

edited

Loading

kiwifb commented Sep 21, 2023 •

edited

Loading