-
-
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
Add support for Python 3.11 and drop support for Python 3.7 #3402
Conversation
setup.py
Outdated
|
||
# Allow overriding the Cython version requirement | ||
CYTHON_STR = os.environ.get('GENSIM_CYTHON_REQUIRES', CYTHON_STR) | ||
|
||
install_requires = [ | ||
NUMPY_STR, | ||
'scipy >= 0.18.1', | ||
'scipy >= 1.9.3', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like the very latest version of scipy.
We want the oldest version (that works) here, because otherwise we make life difficult for users who don't have the very latest.
Could you please find which is the first version of numpy & scipy to support 3.11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please find which is the first version of numpy & scipy to support 3.11?
just check(install on local..pip)
the oldest version that support 3.11 for scipy and numpy are 1.9.2 and 1.23.0 (not very far from the lastest)
maybe we can add special condition just for python 3.11 ?
something like overriding version(numpy,scipy) like the cython https://github.com/RaRe-Technologies/gensim/blob/c93eb0b647ec9de7cfcb43c74c37f295f2944821/setup.py#L330
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://devguide.python.org/versions/, the currently oldest supported Python (before end-of-life) is Python 3.7. So we can ignore Pythons older than that (3.6 etc).
Now looking at SciPy's compatibility matrix https://docs.scipy.org/doc/scipy/dev/toolchain.html :
It seems for 3.11 we need Scipy 1.9.0 (or .1?) and Numpy 1.18.5. Which doesn't match your the oldest version that support 3.11 for scipy and numpy are 1.9.2 and 1.23.0
– can you verify please?
maybe we can add special condition just for python 3.11 ?
I don't think so. Gensim is a library used by ML developers, so we can afford to update its dependencies. We just shouldn't do it without a reason. @mpenkov WDYT?
If the matrix above is correct, I'd go with Scipy>=1.9.0 and Numpy>=1.18.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that's fine. The versions you suggested also make sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah thank you for detail and correction
my missunderstanding
i refer to this issue scipy/scipy#16851 (comment) for oldest "wheel" scipy 1.9.2 and https://numpy.org/news/ for numpy
If the matrix above is correct, I'd go with Scipy>=1.9.0 and Numpy>=1.18.5.
just tried with this version and it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@piskvorky @acul3 Does this mean that 3.6 and 3.7 are no longer supported? If so, we need to update setup.py accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the currently oldest supported Python (before end-of-life) is Python 3.7. So we can ignore Pythons older than that (3.6 etc).
hmm it seems python 3.7 still need supported according to this argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the table above, it looks like it's impossible to support both 3.7 and 3.11 using a single version of scipy, numpy, etc.
So, our options are:
- Use different versions for Py3.11
- Drop 3.7 support now (3.7 reaches EOL in 6 months anyway)
- Wait before 3.7 reaches EOL, and then add support for 3.11
Personally, I think 2) is best. 1) requires a fair bit of effort for very little value (6 more months of 3.7 support, and after 6 months the value of that effort goes down to zero). We can do a major version bump. If people really need support for 3.7, they can continue to use the current version of gensim.
@piskvorky thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agreed. Now that Python versions go EOL frequently, there isn't much expectation to support older ones (not like when py2.7 was supported for ages).
So I'd give preference to the latest / best versions, rather than stability with older.
@mpenkov do we need to update the CI versions too? |
Yes, the CI stuff will need an update. That's probably the bulk of the work for this PR. @acul3 Are you able to take care of this? We need to:
|
sure @mpenkov . i will try work on this today |
there is still open issue on nmslib repo related about this(maybe): some ref also: i tried this also in local(m1) and colab (https://colab.research.google.com/drive/1R-iVww3-4zAuzWrqecsJIG0x1tEUaqLK?usp=sharing)m, give same error any advice? |
@acul3 could you disable that failing test (with nmslib) when running the tests under py3.11? Also remove the autogenerated We're planning to release soon (next week or so) and I'd love to get support for Python 3.11 in! Thanks for the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes! The whitespace reformatting looks sus, otherwise seems OK.
@acul3 Can you please have a look at my changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look goods to me @mpenkov
Merged. Thank you @acul3! |
Update Cython, numpy ,scipy version to support python 3.11
Fixes #3395 fixes #3401.