-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[ci] [python-package] Add CI job testing compatibility with oldest possible versions #5936
Conversation
python ./examples/python-guide/simple_example.py || exit -1 | ||
|
||
echo "" | ||
echo "--- sklearn_example.py ---" |
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.
Why only these examples? Why not any dask
ones or the plotting one?
numpy
, scikit-learn
, pandas
, and scipy
introduce very few additional dependencies. For example, from the CI logs:
installed package versions:
lightgbm @ file:///opt/lgb-build/dist/lightgbm-3.3.5.99-py3-none-manylinux_2_28_x86_64.whl
numpy==1.12.0
pandas==0.24.0
python-dateutil==2.8.2
pytz==2023.3
scikit-learn==0.18.2
scipy==0.19.0
six==1.16.0
dask
, on the other hands, introduces many more. With dask[array,dataframe,distributed]==2.0.0
installed in that same environment, here's what pip freeze
shows:
click==8.0.4
cloudpickle==2.2.1
dask==2.0.0
distributed==2.5.1
HeapDict==1.0.1
importlib-metadata==4.8.3
locket==1.0.0
msgpack==1.0.5
numpy==1.13.0
pandas==0.24.0
partd==1.2.0
psutil==5.9.5
python-dateutil==2.8.2
pytz==2023.3
PyYAML==6.0
scikit-learn==0.18.2
scipy==0.19.0
six==1.16.0
sortedcontainers==2.4.0
tblib==1.7.0
toolz==0.12.0
tornado==6.1
typing_extensions==4.1.1
zict==2.1.0
zipp==3.6.0
I don't want to take on the maintenance burden of making all of those things continue working on Python 3.6.
And as for the plotting examples... similar concern. They rely on some sometimes-hard-to-build graphics libraries like graphviz
and tkinter
.
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.
Why not running the whole test suite? We would need a couple more dependencies like cloudpickle and psutil and some minor changes to some tests but it'd work. That'd allow us to prevent introducing a breaking change for py36.
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.
why not running the whole test suite?
This is exactly what I meant when I said in #5759 (comment) that "I don't want to take on any more support than [a minimal CI job] for Python 3.6". In this project that's already struggling from a severe lack of maintainer attention and availability, I don't think we should be taking on the maintenance burden of trying to keep all the tests working with Python 3.6, which also implies keeping them working with:
- older versions of
pytest
- older versions of all the recursive dependencies of the libraries used by
lightgbm
+ in its tests
In this PR's current state, we get a new CI job that'll confirm that on every commit, the Python package...
- is syntactically-valid Python 3.6 code
- runs without error for the most heavily-used parts of the LightGBM API
I think that's an incremental improvement over the current state, and honestly good enough. I don't support reverting #4891 and trying to say this project fully and completely supports Python 3.6.
If you feel really strongly that we should go farther than this PR does, how about this... could we merge this as-is, and then you could put up a follow-up PR that adds running the unit tests?
According to https://pypistats.org/packages/lightgbm, only around 3-6% of lightgbm
installations from PyPI are for Python 3.6 these days.
I don't know the equivalent number for conda, but it must be lower, given that both Anaconda and conda-forge have stopped supporting Python 3.6 (and even Python 3.7 already!).
Per https://conda.io/projects/conda/en/latest/user-guide/tasks/manage-python.html
Conda supports Python 3.8, 3.9, 3.10, and 3.11.
And per https://conda-forge.org/docs/user/announcements.html#dropping-python-3-7 and conda-forge/conda-forge-pinning-feedstock#2623 conda-forge
dropped support for Python 3.7 in August 2022 (10 months ago).
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 that's an incremental improvement over the current state, and honestly good enough.
Totally agree.
that both Anaconda and conda-forge have stopped supporting Python 3.6 (and even Python 3.7 already!).
This is for publishing new versions of packages, right? You can still create an env with python 3.6 from conda-forge.
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.
You can still create an env with python 3.6 from conda-forge.
Yeah I think you're right. I just tried that on my Mac with mambaforge
:
mamba create --name py36 \
numpy \
pandas \
python=3.6 \
scikit-learn \
scipy
And it worked, albeit with some fairly old versions:
+ ca-certificates 2023.5.7 h8857fd0_0 conda-forge/osx-64 Cached
+ joblib 1.3.0 pyhd8ed1ab_0 conda-forge/noarch Cached
+ libblas 3.9.0 17_osx64_openblas conda-forge/osx-64 Cached
+ libcblas 3.9.0 17_osx64_openblas conda-forge/osx-64 Cached
+ libcxx 16.0.6 hd57cbcb_0 conda-forge/osx-64 Cached
+ libffi 3.4.2 h0d85af4_5 conda-forge/osx-64 Cached
+ libgfortran 5.0.0 11_3_0_h97931a8_31 conda-forge/osx-64 Cached
+ libgfortran5 12.2.0 he409387_31 conda-forge/osx-64 Cached
+ liblapack 3.9.0 17_osx64_openblas conda-forge/osx-64 Cached
+ libopenblas 0.3.23 openmp_h429af6e_0 conda-forge/osx-64 Cached
+ libsqlite 3.42.0 h58db7d2_0 conda-forge/osx-64 Cached
+ libzlib 1.2.13 h8a1eda9_5 conda-forge/osx-64 Cached
+ llvm-openmp 16.0.6 hff08bdf_0 conda-forge/osx-64 Cached
+ ncurses 6.4 hf0c8a7f_0 conda-forge/osx-64 Cached
+ numpy 1.19.5 py36h08b5fde_2 conda-forge/osx-64 Cached
+ openssl 1.1.1u h8a1eda9_0 conda-forge/osx-64 Cached
+ pandas 1.1.5 py36h2be6da3_0 conda-forge/osx-64 Cached
+ pip 21.3.1 pyhd8ed1ab_0 conda-forge/noarch Cached
+ python 3.6.15 haf480d7_0_cpython conda-forge/osx-64 Cached
+ python-dateutil 2.8.2 pyhd8ed1ab_0 conda-forge/noarch Cached
+ python_abi 3.6 2_cp36m conda-forge/osx-64 Cached
+ pytz 2023.3 pyhd8ed1ab_0 conda-forge/noarch Cached
+ readline 8.2 h9e318b2_1 conda-forge/osx-64 Cached
+ scikit-learn 0.24.2 py36h979b75d_1 conda-forge/osx-64 Cached
+ scipy 1.5.3 py36h4f136de_1 conda-forge/osx-64 Cached
+ setuptools 58.0.4 py36h79c6626_2 conda-forge/osx-64 983kB
+ six 1.16.0 pyh6c4a22f_0 conda-forge/noarch Cached
+ sqlite 3.42.0 h2b0dec6_0 conda-forge/osx-64 Cached
+ threadpoolctl 3.1.0 pyh8a188c0_0 conda-forge/noarch Cached
+ tk 8.6.12 h5dbffcc_0 conda-forge/osx-64 Cached
+ wheel 0.37.1 pyhd8ed1ab_0 conda-forge/noarch Cached
+ xz 5.2.6 h775f41a_0 conda-forge/osx-64 Cached
--rm \ | ||
-v $(pwd):/opt/lgb-build \ | ||
-w /opt/lgb-build \ | ||
lightgbm/vsts-agent:manylinux_2_28_x86_64 \ |
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 is the same image we use for building the wheels published to PyPI.
Lines 21 to 22 in ac57d5a
- container: linux-artifact-builder | |
image: lightgbm/vsts-agent:manylinux_2_28_x86_64 |
Lines 36 to 38 in ac57d5a
PRODUCES_ARTIFACTS: 'true' | |
pool: sh-ubuntu | |
container: linux-artifact-builder |
It's also not possible, as of #5759, to run sh build-python.sh
in Python < 3.7
, since:
scikit-build-core
has a floor ofPython >= 3.7
- the oldest supported
setuptools
for Python 3.6,v59.6.0
(December 2021), doesn't supportpyproject.toml
So as of #5759, Python 3.6 users will have to use the published wheels. Customizations like --cuda
will require building the wheel in a Python 3.7+ environment and then distributing it into wherever that Python 3.6 code is running.
I think Python 3.6 users should be expect to face issues for many packages (not just LightGBM) as a result of the changes in the Python packaging ecosystem described in #5061.
Thanks for talking through it with me @jmoralez ! I think we'll be glad we have this. Let's see how it goes. |
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Follow-up from #5759 (comment) (and #5765).
Adds a CI job testing that the Python package is minimally compatible with the oldest version of Python it supports (as indicated by
requires-python
inpyproject.toml
), and the oldest supported versions of its dependencies.