-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Remove support for Python 3.7 #1988
Conversation
When using Python 3.11, some package requires numpy 1.19.4 which may not support Python 3.11. See https://github.com/recommenders-team/recommenders/actions/runs/6079503723/job/16492093035 |
I see two options:
What do you think @SimonYansenZhao? |
@miguelgfierro I tried option 2, but failed, see https://github.com/recommenders-team/recommenders/actions/runs/6079732708/job/16496793669?pr=1988 I'll try python 3.8 - 3.10 in this PR and leave 3.11 in another future PR. |
Some notes from the test results:
|
I asked ChatGPT how to replace papermill and scrapbook, and it looks that there is a dependency that we already have called nbconvert. This would be the code:
what do you think @SimonYansenZhao? |
@SimonYansenZhao @anargyri I'm trying to replicate papermill miguelgfierro/pybase#73 if I do the basic functionality, we can remove 2 deps, and will depend only on nbconvert, which is a library supported by Jupyter, and re. Still WIP. |
@miguelgfierro I think using nbconvert is doable. But now could you help me with the errors in the test result: https://github.com/recommenders-team/recommenders/actions/runs/6116241630/job/16601154561?pr=1988#step:3:2654
|
And I find ploomber may be used to replace scrapbook. Ploomer is built on top of papermill. |
But you have pandas < 1.6 in setup.py, don't you? Or I am missing something? |
@anargyri after I found pandas 2.0 didn't work for scrapbook, I tried to set pandas < 1.6 in setup.py to see if pandas < 1.6 would work. Now scrapbook works with pandas < 1.6, but we cannot use pandas 2.0+ in the future if scrapbook won't upgrade. |
I'm looking into this:
@SimonYansenZhao @anargyri it seems that with python 3.8, we need pandas<2, while with 3.9 we can have pandas>1.5 Trying different pandas versions with different python -> https://github.com/recommenders-team/recommenders/actions/runs/6129325061
So it seems that the error doesn't come from pandas. @SimonYansenZhao have you been able to replicate the error |
I looked at LightGBM https://github.com/microsoft/LightGBM, and they have support for Python 3.7. Wouldn't it be easier if we keep 3.7? Thoughts @SimonYansenZhao @anargyri |
@miguelgfierro I cannot replicate the error |
setup.py
Outdated
"Operating System :: POSIX :: Linux", | ||
], | ||
extras_require=extras_require, | ||
keywords="recommendations recommendation recommenders recommender system engine " | ||
"machine learning python spark gpu", | ||
install_requires=install_requires, | ||
package_dir={"recommenders": "recommenders"}, | ||
python_requires=">=3.6, <3.10", | ||
python_requires=">=3.8, <=3.9", |
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.
python_requires=">=3.8, <=3.9", | |
python_requires=">=3.6", |
@SimonYansenZhao we had a discussion in the reco meeting. There are a couple of things we are proposing:
- leave python requires with >= 3.6 so people can install recommenders with any version of python. Right now, for example, people can't install the pypi package on 3.10 Then we say that we officially only support 3.8-3.10. With that we avoid issues like [ASK] No module named 'recommenders' after installing recommenders in Colab #1928 where people can't install recommenders on versions that are not in the requires, and at the same time, we officially only tests the versions we support.
- Maybe it would be easier to split this PR into 2, first we remove the support of 3.7, and then in another PR we add 3.10.
What do you think?
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.
OK.
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
is_sequence() is deleted in TensorFlow 2.13.0. See https://github.com/tensorflow/tensorflow/releases/tag/v2.13.0 Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
229a3bf
to
1c6a1ff
Compare
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
…der.tokenize_text() Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
@miguelgfierro @anargyri I find As discussed in #1988 (comment), I only dropped the support for Python 3.7 in this PR, and will create another PR for adding support for 3.10. |
No, we can drop it |
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.
I think we should change python_requires=">=3.6"
, as of this comment: #1988 (comment) or if we don't like 3.6, we can do 3.7+
Apart from that, this is great
Description
This PR dropped the support for Python 3.7 since it'll reach the end of life on Jun 27, 2023.
This PR is moved from #1937, because of the sign-off requirement.
Related Issues
References
Checklist:
staging branch
and not tomain branch
.