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

MNT Use a minimal Windows Docker container to check the wheels #18802

Merged
merged 124 commits into from
Nov 18, 2020

Conversation

alfaro96
Copy link
Member

@alfaro96 alfaro96 commented Nov 9, 2020

Reference Issues/PRs

Closes MacPython/scikit-learn-wheels#44.

What does this implement/fix? Explain your changes.

This PR adds a job to check the wheels in a minimal Windows Docker container.

Any other comments

Note that the base image in the Windows Dockerfile (Windows) is minimal in the sense that it only installs Python from https://python.org (Step 7) and pip using the get-pip.py script (Step 9). Using this minimal base image we install the corresponding Windows wheel and import sklearn for checking DLL errors (#15899).

CC @ogrisel and @thomasjpfan.

@alfaro96 alfaro96 marked this pull request as ready for review November 10, 2020 10:37
@alfaro96 alfaro96 changed the title [WIP] MNT Use a minimal Windows Docker container to check the wheels MNT Use a minimal Windows Docker container to check the wheels Nov 10, 2020
@ogrisel
Copy link
Member

ogrisel commented Nov 18, 2020

It seems that we still have problems with the find command (although rarer):

https://github.com/scikit-learn/scikit-learn/runs/1418596447

@alfaro96
Copy link
Member Author

alfaro96 commented Nov 18, 2020

Waiting for a green :)

Edit: Green in the Source distribution job and it seems that the network tests are properly skipped.

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM, let's merge!

@ogrisel ogrisel merged commit 2f09bbb into scikit-learn:master Nov 18, 2020
@ogrisel
Copy link
Member

ogrisel commented Nov 18, 2020

Thank you very much for fixing this @alfaro96 !

@ogrisel
Copy link
Member

ogrisel commented Nov 18, 2020

Can you please open an issue not to forget about #18802 (comment) .

@thomasjpfan
Copy link
Member

This is awesome work @alfaro96 @ogrisel !

@alfaro96 alfaro96 deleted the minimal_windows_container branch November 18, 2020 18:08
@alfaro96
Copy link
Member Author

alfaro96 commented Nov 18, 2020

I would not say that it was easy, but I am really happy with the result (and thank you @ogrisel for your work and reviews).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a minimal Windows docker container to check the wheels
3 participants