Skip to content
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 Add wheel builds for Python 3.11 #24446

Merged
merged 51 commits into from Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
99c99b9
[cd build] Add wheel builds for python 3.11
cmarmo Sep 13, 2022
99f2f36
[cd build] Fix MacOS builds setting SETUPTOOLS_USE_DISTUTILS=stdlib.
cmarmo Sep 15, 2022
a005d02
Remove relic.
cmarmo Sep 15, 2022
18a1807
[cd build] Vendor msvcp140.dll.
cmarmo Sep 17, 2022
f7923b1
Apply black
cmarmo Sep 17, 2022
a75bb7d
Merge branch 'main' into wheel-python-311
cmarmo Sep 17, 2022
0b42e61
Merge branch 'main' into wheel-python-311
cmarmo Sep 17, 2022
d073620
Merge branch 'main' into wheel-python-311
cmarmo Sep 23, 2022
6add9af
[cd build] Trigger build.
cmarmo Sep 23, 2022
b69242c
Merge branch 'main' into wheel-python-311
jjerphan Sep 28, 2022
bce8672
[cd build] Merge branch 'main' into wheel-python-311
cmarmo Sep 29, 2022
6c1a129
Merge branch 'main' into wheel-python-311
jjerphan Oct 7, 2022
a48568b
[cd build] Trigger CD since #24541 is merged
jjerphan Oct 7, 2022
2f411a9
Apply suggestions from code review
cmarmo Oct 8, 2022
5345e01
Merge branch 'main' into wheel-python-311
cmarmo Oct 8, 2022
7e3af0a
Clarify comments.
cmarmo Oct 8, 2022
4c47cb6
[cd build] Install pandas wheels on windows 64 (available since the e…
cmarmo Oct 8, 2022
00dbfd8
[cd build] scipy 1.9.2 wheels for 3.11 published on pypi (except for …
cmarmo Oct 9, 2022
cbbf87e
[cd build] scipy 1.9.2 wheels for 3.11 published on pypi.
cmarmo Oct 9, 2022
b77b6a3
[cd build] force scipy version for win32.
cmarmo Oct 9, 2022
40c1d03
[cd build] force scipy version for win32.
cmarmo Oct 9, 2022
45cee88
[cd build] force scipy version for win32.
cmarmo Oct 9, 2022
1816933
[cd build] force scipy version for win32.
cmarmo Oct 9, 2022
26a41ab
[cd build] Experimenting with PowerShell.
cmarmo Oct 9, 2022
18eac63
[cd build] Experimenting with PowerShell.
cmarmo Oct 9, 2022
0c4e80d
[cd build] Experimenting with PowerShell.
cmarmo Oct 9, 2022
c51315e
[cd build] Experimenting with PowerShell.
cmarmo Oct 9, 2022
fca7ca6
[cd build] Set dependence in pyproject.toml.
cmarmo Oct 9, 2022
65dbb59
[cd build] Set dependence in pyproject.toml.
cmarmo Oct 9, 2022
f0938db
[cd build] Set dependence in pyproject.toml.
cmarmo Oct 9, 2022
b4b6f5f
[cd build] Install scipy 1.9.1 for tests.
cmarmo Oct 9, 2022
2d2baed
[cd build] Using platform_machine.
cmarmo Oct 9, 2022
2d1cd7f
[cd build] No way to use pyproject.toml deendencies.
cmarmo Oct 9, 2022
82a21b6
[cd build] Prefer binaries when building.
cmarmo Oct 10, 2022
33fe747
[cd build] Prefer binaries when building (via PIP_BUILD_OPTIONS).
cmarmo Oct 10, 2022
9950435
[cd build] Prefer binaries when building (via PIP_BUILD_OPTIONS).
cmarmo Oct 10, 2022
fd2371f
[ci skip] Revert pip build options.
cmarmo Oct 10, 2022
8fe7c36
Merge branch 'main' into wheel-python-311
cmarmo Oct 11, 2022
21e769d
[cd build] Trig wheel build.
cmarmo Oct 11, 2022
3210043
[cd build] Add condition on extra index.
cmarmo Oct 11, 2022
657b563
[cd build] Add condition on extra index.
cmarmo Oct 11, 2022
6db7e7c
Reformat condition. Define PIP_PRE.
cmarmo Oct 12, 2022
c9a59e2
Merge branch 'main' into wheel-python-311
cmarmo Oct 12, 2022
ed2c6a0
Update .github/workflows/wheels.yml
cmarmo Oct 12, 2022
a3a8100
Merge branch 'main' into wheel-python-311
cmarmo Oct 14, 2022
5a43727
[cd build gh] Update vendor.py, remove bitness argument, add PIP_PRE …
cmarmo Oct 14, 2022
66ab6de
Merge branch 'main' into wheel-python-311
cmarmo Oct 17, 2022
75e53eb
[cd build] Add 3.11 wheel for arm64-graviton on travis.
cmarmo Oct 17, 2022
b12be0b
Merge branch 'main' into wheel-python-311
cmarmo Oct 18, 2022
a1e9d7d
[cd build] Merge branch 'main' into wheel-python-311
cmarmo Oct 20, 2022
1c50c26
[cd build] Address comments.
cmarmo Oct 20, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 27 additions & 15 deletions .github/workflows/wheels.yml
Expand Up @@ -57,63 +57,66 @@ jobs:
# https://github.com/scikit-learn/scikit-learn/issues/22530
- os: windows-2019
python: 38
bitness: 64
platform_id: win_amd64
- os: windows-latest
python: 39
bitness: 64
platform_id: win_amd64
- os: windows-latest
python: 310
bitness: 64
platform_id: win_amd64
- os: windows-latest
python: 311
platform_id: win_amd64

# Linux 64 bit manylinux2014
- os: ubuntu-latest
python: 38
bitness: 64
platform_id: manylinux_x86_64
manylinux_image: manylinux2014
- os: ubuntu-latest
python: 39
bitness: 64
platform_id: manylinux_x86_64
manylinux_image: manylinux2014

# NumPy on Python 3.10 only supports 64bit and is only available with manylinux2014
- os: ubuntu-latest
python: 310
bitness: 64
platform_id: manylinux_x86_64
manylinux_image: manylinux2014

# NumPy on Python 3.11 only supports 64bit and is only available with manylinux2014
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
- os: ubuntu-latest
python: 311
platform_id: manylinux_x86_64
manylinux_image: manylinux2014

# MacOS x86_64
- os: macos-latest
bitness: 64
python: 38
platform_id: macosx_x86_64
- os: macos-latest
bitness: 64
python: 39
platform_id: macosx_x86_64
- os: macos-latest
bitness: 64
python: 310
platform_id: macosx_x86_64
- os: macos-latest
python: 311
platform_id: macosx_x86_64

# MacOS arm64
- os: macos-latest
bitness: 64
python: 38
platform_id: macosx_arm64
- os: macos-latest
bitness: 64
python: 39
platform_id: macosx_arm64
- os: macos-latest
bitness: 64
python: 310
platform_id: macosx_arm64
- os: macos-latest
python: 311
platform_id: macosx_arm64

steps:
- name: Checkout scikit-learn
Expand All @@ -124,6 +127,13 @@ jobs:
with:
python-version: '3.9' # update once build dependencies are available

- name: Set pip extra options
if: github.event_name == 'schedule'
run: |
# Test with nightly dependencies for nightly builds
echo "PIP_EXTRA_INDEX=https://pypi.anaconda.org/scipy-wheels-nightly/simple" >> $GITHUB_ENV
echo "PIP_PRE=1" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we do that only for the 3.11 build? The other builds should work with the released version of numpy / scipy (once scipy/scipy#17224 backported as part of scipy 1.9.3).

Copy link
Member Author

@cmarmo cmarmo Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not completely aware of the scipy wheels publication procedure.
In my opinion if it had been possible to test scikit-learn nightly wheels against scipy nightly wheels the failure in #24612 and the one scipy/scipy#17224 is willing to fix (both related to 3.10 too), would have been detected before the release of scipy 1.9.2 , and the backporting would not have been necessary.
The question is how far do you want to go with testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that SciPy 1.9.3 has been released, I think we need to remove this step from this PR if we want to publish stable wheels once Python 3.11 is officially release.

We in the meantime create another PR for adapting scikit-learn nightly builds' dependences (incorporating this step in if needed). What do you think?

Copy link
Member

@ogrisel ogrisel Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @cmarmo I had missed your reply. Indeed the testing against the nightly builds of our dependencies is a different goal than publishing our own nightly built wheels for project that depend upon us. I think that for the latter we should build always build the wheels we publish against stable dependencies not have our wheel publishing infrastructure blocked by a bug/regression in the dev branch of one of our dependencies.

But I agree with your suggestions and @jjerphan's that we might want to upgrade our nightly [scipy-dev] tests (currently configured on Azure pipelines and only for linux) to also run the tests on all the platforms, probably by reusing the config done on github actions for both goals (testing against dev dependencies and publishing our own dev wheels).

But let's do that in a dedicated follow-up PR.

Copy link
Member

@ogrisel ogrisel Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in https://github.com/scikit-learn/scikit-learn/pull/24446/files#r1000353644, let's focus this PR on building and publishing 3.11 wheels against stable dependencies and remove the use of dev dependencies from this PR.

We can tackle nightly testing against dev dependencies (without uploading) in a dedicated PR.

To do that, we could introduce a second github actions scheduled event with a specific env variable/config flag to also disable the final upload to anaconda or pypi.

We will also have to re-implement the [scipy-dev] commit message trigger currently implemented in our Azure Pipelines config and delete that config from Azure Pipelines.

Suggested change
- name: Set pip extra options
if: github.event_name == 'schedule'
run: |
# Test with nightly dependencies for nightly builds
echo "PIP_EXTRA_INDEX=https://pypi.anaconda.org/scipy-wheels-nightly/simple" >> $GITHUB_ENV
echo "PIP_PRE=1" >> $GITHUB_ENV

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmarmo I did not have time to write that comment and you addressed it concurrently :)


- name: Build and test wheels
env:
CONFTEST_PATH: ${{ github.workspace }}/conftest.py
Expand All @@ -132,16 +142,18 @@ jobs:
OPENBLAS_NUM_THREADS=2
SKLEARN_SKIP_NETWORK_TESTS=1
SKLEARN_BUILD_PARALLEL=3
PIP_EXTRA_INDEX_URL=${{ env.PIP_EXTRA_INDEX }}
PIP_PRE=${{ env.PIP_PRE }}
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
CIBW_BUILD: cp${{ matrix.python }}-${{ matrix.platform_id }}
CIBW_ARCHS: all
CIBW_MANYLINUX_X86_64_IMAGE: ${{ matrix.manylinux_image }}
CIBW_MANYLINUX_I686_IMAGE: ${{ matrix.manylinux_image }}
CIBW_TEST_SKIP: "*-macosx_arm64"
CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: bash build_tools/github/repair_windows_wheels.sh {wheel} {dest_dir} ${{ matrix.bitness }}
CIBW_BEFORE_TEST_WINDOWS: bash build_tools/github/build_minimal_windows_image.sh ${{ matrix.python }} ${{ matrix.bitness }}
CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: bash build_tools/github/repair_windows_wheels.sh {wheel} {dest_dir}
CIBW_BEFORE_TEST_WINDOWS: bash build_tools/github/build_minimal_windows_image.sh ${{ matrix.python }}
CIBW_TEST_REQUIRES: pytest pandas threadpoolctl
CIBW_TEST_COMMAND: bash {project}/build_tools/github/test_wheels.sh
CIBW_TEST_COMMAND_WINDOWS: bash {project}/build_tools/github/test_windows_wheels.sh ${{ matrix.python }} ${{ matrix.bitness }}
CIBW_TEST_COMMAND_WINDOWS: bash {project}/build_tools/github/test_windows_wheels.sh ${{ matrix.python }}
CIBW_BUILD_VERBOSITY: 1

run: bash build_tools/github/build_wheels.sh
Expand Down
4 changes: 4 additions & 0 deletions build_tools/github/Windows
Expand Up @@ -5,10 +5,14 @@ FROM winamd64/python:$PYTHON_VERSION-windowsservercore
ARG WHEEL_NAME
ARG CONFTEST_NAME
ARG CIBW_TEST_REQUIRES
ARG PIP_EXTRA_INDEX_URL
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
ARG PIP_PRE

# Copy and install the Windows wheel
COPY $WHEEL_NAME $WHEEL_NAME
COPY $CONFTEST_NAME $CONFTEST_NAME
RUN echo PIP_EXTRA_INDEX_URL = $env:PIP_EXTRA_INDEX_URL
Copy link
Member

@thomasjpfan thomasjpfan Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this sets the variables for pip to pick up later

Suggested change
RUN echo PIP_EXTRA_INDEX_URL = $env:PIP_EXTRA_INDEX_URL
RUN export PIP_EXTRA_INDEX_URL=$env:PIP_EXTRA_INDEX_URL
RUN export PIP_PRE=$env:PIP_PRE

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your comment, but the script is working eg here, the variable is set: do you mind explaining to me what is the difference with the export command? Sorry for bothering and thanks for your patience.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @cmarmo: using ARG should be enough to set variables.

RUN echo PIP_PRE = $env:PIP_PRE
RUN pip install $env:WHEEL_NAME

# Install the testing dependencies
Expand Down
18 changes: 10 additions & 8 deletions build_tools/github/build_minimal_windows_image.sh
Expand Up @@ -4,14 +4,6 @@ set -e
set -x

PYTHON_VERSION=$1
BITNESS=$2

if [[ "$BITNESS" == "32" ]]; then
# 32-bit architectures are not supported
# by the official Docker images: Tests will just be run
# on the host (instead of the minimal Docker container).
exit 0
fi

TEMP_FOLDER="$HOME/AppData/Local/Temp"
WHEEL_PATH=$(ls -d $TEMP_FOLDER/**/*/repaired_wheel/*)
Expand All @@ -22,10 +14,20 @@ cp $WHEEL_PATH $WHEEL_NAME
# Dot the Python version for identyfing the base Docker image
PYTHON_VERSION=$(echo ${PYTHON_VERSION:0:1}.${PYTHON_VERSION:1:2})

# TODO: Remove when 3.11 images will be available for
# windows (for now the docker image is tagged as 3.11-rc)
# TODO: Remove when 3.11 images will be available for
# scipy (for now only available from nightly anaconda index)
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
if [[ "$PYTHON_VERSION" == "3.11" ]]; then
PYTHON_VERSION=$(echo ${PYTHON_VERSION}-rc)
fi

# Build a minimal Windows Docker image for testing the wheels
docker build --build-arg PYTHON_VERSION=$PYTHON_VERSION \
--build-arg WHEEL_NAME=$WHEEL_NAME \
--build-arg CONFTEST_NAME=$CONFTEST_NAME \
--build-arg CIBW_TEST_REQUIRES="$CIBW_TEST_REQUIRES" \
--build-arg PIP_EXTRA_INDEX_URL="$PIP_EXTRA_INDEX_URL" \
ogrisel marked this conversation as resolved.
Show resolved Hide resolved
--build-arg PIP_PRE="$PIP_PRE" \
-f build_tools/github/Windows \
-t scikit-learn/minimal-windows .
5 changes: 5 additions & 0 deletions build_tools/github/build_wheels.sh
Expand Up @@ -31,6 +31,11 @@ if [[ "$RUNNER_OS" == "macOS" ]]; then
export CFLAGS="$CFLAGS -I$PREFIX/include"
export CXXFLAGS="$CXXFLAGS -I$PREFIX/include"
export LDFLAGS="$LDFLAGS -Wl,-rpath,$PREFIX/lib -L$PREFIX/lib -lomp"
# Disable the use of setuptools's vendored copy distutils when invoking setuptools
# See: https://setuptools.pypa.io/en/latest/deprecated/distutils-legacy.html
# TODO: remove the definition of this environment variable when no
# reference to distutils exist in the code-base for building scikit-learn.
export SETUPTOOLS_USE_DISTUTILS=stdlib
cmarmo marked this conversation as resolved.
Show resolved Hide resolved
fi

# The version of the built dependencies are specified
Expand Down
3 changes: 1 addition & 2 deletions build_tools/github/repair_windows_wheels.sh
Expand Up @@ -5,12 +5,11 @@ set -x

WHEEL=$1
DEST_DIR=$2
BITNESS=$3

# By default, the Windows wheels are not repaired.
# In this case, we need to vendor VCRUNTIME140.dll
wheel unpack "$WHEEL"
WHEEL_DIRNAME=$(ls -d scikit_learn-*)
python build_tools/github/vendor.py "$WHEEL_DIRNAME" "$BITNESS"
python build_tools/github/vendor.py "$WHEEL_DIRNAME"
wheel pack "$WHEEL_DIRNAME" -d "$DEST_DIR"
rm -rf "$WHEEL_DIRNAME"
1 change: 0 additions & 1 deletion build_tools/github/test_windows_wheels.sh
Expand Up @@ -4,7 +4,6 @@ set -e
set -x

PYTHON_VERSION=$1
BITNESS=$2

docker container run \
--rm scikit-learn/minimal-windows \
Expand Down
39 changes: 16 additions & 23 deletions build_tools/github/vendor.py
Expand Up @@ -114,18 +114,18 @@ def make_distributor_init_64_bits(
)


def main(wheel_dirname, bitness):
def main(wheel_dirname):
"""Embed vcomp140.dll, vcruntime140.dll and vcruntime140_1.dll."""
if not op.exists(VCOMP140_SRC_PATH):
raise ValueError(f"Could not find {VCOMP140_SRC_PATH}.")

if not op.exists(VCRUNTIME140_SRC_PATH):
raise ValueError(f"Could not find {VCRUNTIME140_SRC_PATH}.")

if not op.exists(VCRUNTIME140_1_SRC_PATH) and bitness == "64":
if not op.exists(VCRUNTIME140_1_SRC_PATH):
raise ValueError(f"Could not find {VCRUNTIME140_1_SRC_PATH}.")

if not op.exists(MSVCP140_SRC_PATH) and bitness == "64":
if not op.exists(MSVCP140_SRC_PATH):
raise ValueError(f"Could not find {MSVCP140_SRC_PATH}.")

if not op.isdir(wheel_dirname):
Expand All @@ -149,29 +149,22 @@ def main(wheel_dirname, bitness):
print(f"Copying {VCRUNTIME140_SRC_PATH} to {target_folder}.")
shutil.copy2(VCRUNTIME140_SRC_PATH, target_folder)

if bitness == "64":
print(f"Copying {VCRUNTIME140_1_SRC_PATH} to {target_folder}.")
shutil.copy2(VCRUNTIME140_1_SRC_PATH, target_folder)
print(f"Copying {VCRUNTIME140_1_SRC_PATH} to {target_folder}.")
shutil.copy2(VCRUNTIME140_1_SRC_PATH, target_folder)

print(f"Copying {MSVCP140_SRC_PATH} to {target_folder}.")
shutil.copy2(MSVCP140_SRC_PATH, target_folder)
print(f"Copying {MSVCP140_SRC_PATH} to {target_folder}.")
shutil.copy2(MSVCP140_SRC_PATH, target_folder)

# Generate the _distributor_init file in the source tree
print("Generating the '_distributor_init.py' file.")
if bitness == "32":
make_distributor_init_32_bits(
distributor_init, vcomp140_dll_filename, vcruntime140_dll_filename
)
else:
make_distributor_init_64_bits(
distributor_init,
vcomp140_dll_filename,
vcruntime140_dll_filename,
vcruntime140_1_dll_filename,
msvcp140_dll_filename,
)
make_distributor_init_64_bits(
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
distributor_init,
vcomp140_dll_filename,
vcruntime140_dll_filename,
vcruntime140_1_dll_filename,
msvcp140_dll_filename,
)


if __name__ == "__main__":
_, wheel_file, bitness = sys.argv
main(wheel_file, bitness)
_, wheel_file = sys.argv
main(wheel_file)