-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
Switch to Cython language level 3 #3344
Switch to Cython language level 3 #3344
Conversation
Python 2 is not supported by gensim, so switching to language level 3 should be fine. Silences a warning from Cython: FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release!
Thanks. Where is that warning shown? If some extensions are being built with an incorrect language level, that sounds dangerous. CI is failing though. |
The warning is shown during the build of the Debian package of gensim (amd64 log). Side note: the log also shows various GCC warnings, some test warnings and a warning about setup.py being deprecated within the Python community. Unfortunately based on the CI results for this PR, it looks like the Cython code in gensim isn't yet compatible with Cython language version 3, so I'm going to mark this PR as draft and work on fixing the issues as I get time. |
Hmm, maybe I should instead mark those files as language level 2 for now, to silence the warnings. |
So we should have |
I thought so too, but the CI fails when I add that everywhere.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
Hmm, the CI errors complain about syntax stuff in the Cython code like |
The problem seems to stem from this code: which seems indeed syntactically broken:
The code comes from 3c3506d in #2127. Or is it some special Cython syntax for PXD? I don't get it. CC @persiyanov @mpenkov – any idea why Cython compilation with |
My hypothesis: error is related to the compilation order of Gensim extensions.
Checking my hypothesis now :) |
OK I think I figured it out. The issue is with relative imports ( @pabs3 I'll push the fix to your branch, OK? |
I usually disable maintainer edits of my patches; had
some suboptimal experiences with that in the past.
I think it would be best to merge your patch first, since otherwise git
bisects will have to skip my patch since it will be unbuildable, but if
your patch goes in first, then both patches will be bisect buildable.
Anyway, I've added your patch to the top of my branch too.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
Thank you @pabs3 ! |
Python 2 is not supported by gensim, so
switching to language level 3 should be fine.
Silences a warning from Cython:
FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release!