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

BLD Build 3.10 wheel [cd build gh] #21232

Merged
merged 35 commits into from Dec 2, 2021
Merged

Conversation

@thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Oct 3, 2021

Fixes #21401

Now that 3.10 wheels for numpy and scipy are in https://pypi.anaconda.org/scipy-wheels-nightly/simple, this PR enables us to build scikit-learn wheels with them.

Note that numpy and scipy is on manylinux2014.

@thomasjpfan thomasjpfan marked this pull request as ready for review Oct 4, 2021
pyproject.toml Outdated
@@ -3,7 +3,7 @@
requires = [
"setuptools",
"wheel",
"Cython>=0.28.5",
"Cython>=0.28.5,<3.0",
Copy link
Member Author

@thomasjpfan thomasjpfan Oct 4, 2021

Choose a reason for hiding this comment

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

Needed this to stop Cython 3.0 from installing with PIP_PRE=true

PIP_PRE=true is required because scipy 3.10 is only avaliable as a dev release in https://pypi.anaconda.org/scipy-wheels-nightly/simple

# Comment out when numpy/scipy dependencies are on pypi
- name: Set pypi index for python dev
if: matrix.python == 310
run: |
echo "EXTRA_CIBW_ENVIRONMENT=PIP_EXTRA_INDEX_URL=https://pypi.anaconda.org/scipy-wheels-nightly/simple PIP_PRE=true" >> $GITHUB_ENV
# Do not use the dev version of pandas
echo "PANDAS_VERSION_SPECIFIER=<1.4" >> $GITHUB_ENV
Copy link
Member Author

@thomasjpfan thomasjpfan Oct 4, 2021

Choose a reason for hiding this comment

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

When a new version of Python come up, I expect the builds for the Python RC to be in https://pypi.anaconda.org/scipy-wheels-nightly/simple first, so setting PIP_PRE=true is required to automatically get the dev builds.

Currently there is a mix of Python 3.10 wheels on scipy-wheels-nightly and on pypi. numpy and pandas is on pypi and scipy is on scipy-wheels-nightly.

@thomasjpfan thomasjpfan added this to the 1.0.1 milestone Oct 4, 2021
@thomasjpfan
Copy link
Member Author

@thomasjpfan thomasjpfan commented Oct 4, 2021

Marked for 1.0.1 so we can have Python 3.10 builds for 1.0.X

@@ -7,6 +7,7 @@ COMMIT_MSG=$(git log --no-merges -1 --oneline)

# The commit marker "[cd build]" will trigger the build when required
if [[ "$GITHUB_EVENT_NAME" == schedule ||
"$COMMIT_MSG" =~ \[cd\ build\] ]]; then
"$COMMIT_MSG" =~ \[cd\ build\] ||
"$COMMIT_MSG" =~ \[cd\ build\ gh\] ]]; then
Copy link
Member

@ogrisel ogrisel Oct 5, 2021

Choose a reason for hiding this comment

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

nice :)

Copy link
Member

@adrinjalali adrinjalali Nov 18, 2021

Choose a reason for hiding this comment

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

do we have this documented?

Copy link
Member

@ogrisel ogrisel Dec 2, 2021

Choose a reason for hiding this comment

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

now we do (as part of #21849).

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Oct 5, 2021

It nice to see that it works but shouldn't we wait for numpy and scipy to be release for Python 3.10 before merging this with a simpler config that does not rely on the dev wheels?

@thomasjpfan thomasjpfan removed this from the 1.0.1 milestone Oct 5, 2021
@thomasjpfan thomasjpfan marked this pull request as draft Oct 5, 2021
@thomasjpfan
Copy link
Member Author

@thomasjpfan thomasjpfan commented Oct 5, 2021

It nice to see that it works but shouldn't we wait for numpy and scipy to be release for Python 3.10 before merging this with a simpler config that does not rely on the dev wheels?

I agree. I'll place this PR into draft mode and take it out of draft when numpy/scipy wheels are on pypi.

@thomasjpfan
Copy link
Member Author

@thomasjpfan thomasjpfan commented Oct 22, 2021

Still waiting on MacPython/scipy-wheels#132

@thomasjpfan
Copy link
Member Author

@thomasjpfan thomasjpfan commented Nov 8, 2021

Opened MacPython/scipy-wheels#139 to fix the bug in scipy.

@thomasjpfan
Copy link
Member Author

@thomasjpfan thomasjpfan commented Nov 8, 2021

SciPy on Windows & Python 3.10 is now fixed on PyPi. This PR should be good to go.

@ulupo
Copy link
Contributor

@ulupo ulupo commented Nov 17, 2021

Hi! Thanks to the scikit-learn team for being so quick at work with the Python 3.10 wheels. Just wondering, for the purposes of a package I maintain which has scikit-learn as a dependency, whether there is a target timeline for merging this PR and for releasing the wheels on PyPI?

@thomasjpfan thomasjpfan changed the title BLD Build 3.10 wheel for linux [cd build gh] BLD Build 3.10 wheel [cd build gh] Nov 18, 2021
Copy link
Member

@adrinjalali adrinjalali left a comment

Thanks @thomasjpfan

@@ -7,6 +7,7 @@ COMMIT_MSG=$(git log --no-merges -1 --oneline)

# The commit marker "[cd build]" will trigger the build when required
if [[ "$GITHUB_EVENT_NAME" == schedule ||
"$COMMIT_MSG" =~ \[cd\ build\] ]]; then
"$COMMIT_MSG" =~ \[cd\ build\] ||
"$COMMIT_MSG" =~ \[cd\ build\ gh\] ]]; then
Copy link
Member

@adrinjalali adrinjalali Nov 18, 2021

Choose a reason for hiding this comment

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

do we have this documented?

build_tools/github/check_wheels.py Outdated Show resolved Hide resolved
Copy link
Member

@ogrisel ogrisel left a comment

LGTM once the remaining review comments are dealt with. Thanks very much @thomasjpfan!

.travis.yml Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
@ogrisel
Copy link
Member

@ogrisel ogrisel commented Nov 19, 2021

Hi! Thanks to the scikit-learn team for being so quick at work with the Python 3.10 wheels. Just wondering, for the purposes of a package I maintain which has scikit-learn as a dependency, whether there is a target timeline for merging this PR and for releasing the wheels on PyPI?

Once merged, will need to backport this to the 1.0.X branch. From there we could either build some Python 3.10 wheels for 1.0.1 and upload them manually to pypi or we could wait for 1.0.2 which should happen soon-ish.

@ulupo
Copy link
Contributor

@ulupo ulupo commented Nov 19, 2021

Thanks @ogrisel and @thomasjpfan !

ogrisel
ogrisel approved these changes Dec 2, 2021
Copy link
Member

@ogrisel ogrisel left a comment

LGTM assuming CI stays green after the merge conflicts resolution. If #21827 is merged first we will need to add the Python 3.10 entry to macos/arm64 here before merging.

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Dec 2, 2021

The unrelated Azure Pipelines failures on Windows has already been reported here: #21798.

@ogrisel ogrisel merged commit 8876439 into scikit-learn:main Dec 2, 2021
53 checks passed
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 2, 2021

Nice.

glemaitre added a commit to glemaitre/scikit-learn that referenced this issue Dec 24, 2021
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
glemaitre added a commit that referenced this issue Dec 25, 2021
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants