Skip to content

Fix build with Cython 3.1 #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
alejo7797 wants to merge 3 commits into from
Closed

Fix build with Cython 3.1 #3

alejo7797 wants to merge 3 commits into from

Conversation

@alejo7797
Copy link

@alejo7797 alejo7797 commented Sep 2, 2025

I tried my hand at getting CyPari to build (and to pass its tests) under Cython 3.1.
This comes at the cost of dropping Python 2 compatibility (if that was still a thing).
Most of this just came from looking at cypari2 and what changes they made.

See cython/cython#5870, cython/cython@9a22b49 for the root of these changes.

@alejo7797 alejo7797 mentioned this pull request Sep 2, 2025
Merged
13 tasks
@culler
Copy link
Member

culler commented Sep 2, 2025

Thanks.

I won't be able to look at this for a few days.

The Windows build failure looks like something Pari knows how to deal with - they know the size of a long in Windows. So I am guessing some small configure tweak will fix it.

@NathanDunfield
Copy link
Member

NathanDunfield commented Sep 3, 2025

@culler wrote:

The Windows build failure looks like something Pari knows how to deal with - they know the size of a long in Windows. So I am guessing some small configure tweak will fix it.

The windows failure is not caused by this PR, it is a pre-existing condition.

@NathanDunfield
Copy link
Member

NathanDunfield commented Jan 1, 2026 •
edited
Loading

The Windows CI is now working again for the main branch, so I reran it on this PR. It fails, perhaps due to PARI's doing #def long long long on Windows. The key lines of the log seem to be:

D:\a\CyPari\CyPari\cypari\pylong_support.h(108): warning C4005: 'LONG_MAX': macro redefinition
D:\a\CyPari\CyPari\libcache\pari\include\pari\parigen.h(33): note: see previous definition of 'LONG_MAX'
D:\a\CyPari\CyPari\cypari\pylong_support.h(109): warning C4005: 'LONG_MIN': macro redefinition
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\limits.h(38): note: see previous definition of 'LONG_MIN'
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\immintrin.h(105): error C2632: '__int64' followed by 'long' is illegal
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\immintrin.h(1260): error C2632: '__int64' followed by 'long' is illegal

@culler do you have any suggestions?

@culler
Copy link
Member

culler commented Jan 1, 2026

I think we can ignore the errors about redefining LONG_MIN and LONG_MAX. But the other errors are definitely the result of the crazy #define long long long. I think the thing to do is to figure out where that header is being #included and bracket the #include statement with #undef long and #define long long long. Unfortunately, that will probably require patching _pari.c, with some hack along the lines of what we did in setup.py before.

@alejo7797
Remove Gen.__cmp__ (use cmp instead)
fe0bdb5 
Cython 3.1 no longer discards extension types' `__cmp__` method. However
`cmp()` is not a thing in Python 3, at the very least the doctests have
to be modified/removed. Sage's `cypari2` opted for the following.

See
cython/cython@9a22b49.

Based off of
sagemath/cypari2@1db13c9.
@alejo7797
Remove Gen.__hex__ tests
2981153 
Cython 3.1 no longer discards extension types' `__hex__` method.
This would mean these tests get run. But `hex` does not use `.__hex__`
in Python 3 so these tests aren't actually testing the function
definition they live under.

Sage's `cypari2` also has no tests for `__hex__`.

See
cython/cython@9a22b49.
alejo7797 added a commit to alejo7797/CyPari that referenced this pull request Jan 5, 2026
@alejo7797
Change gen_to_integer to use PyLong_FromLong
d8e0485 
This is what is left of a commit from PR
3-manifolds#3 after rebasing.
@alejo7797
Change gen_to_integer to use PyLong_FromLong
0206edc 
This is what is left of a commit from PR
3-manifolds#3 after rebasing.
@alejo7797 alejo7797 marked this pull request as draft January 5, 2026 22:50
@culler
Copy link
Member

culler commented Jan 5, 2026

Hi Alex,

Thank you very much for working on this. We truly do appreciate your efforts! However, I think we have now resolved this issue. (As of yesterday, and it was quite technical.) The current CI runs are using the latest Cython.

@alejo7797
Copy link
Author

alejo7797 commented Jan 5, 2026

Oh that's great to hear! I somehow missed that before rebasing. I'll go ahead and close this then.

@alejo7797 alejo7797 closed this Jan 5, 2026
@alejo7797 alejo7797 deleted the update-cython branch January 5, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alejo7797 @culler @NathanDunfield