Skip to content

Conversation

@eddiebergman
Copy link
Contributor

@eddiebergman eddiebergman commented Dec 14, 2021

This PR builds wheels for the same distributions as supported in sklearn github actions.

This action also runs our suite of tests on these wheels which seems to complete without error.

The output of the test can be seen here:

This also adds the workflow_dispatch event so that it can be triggered manually.

I would still advocate for some manual testing of the wheels produced in the artifacts on a seperate windows/mac device, seperate from the environment it was built on, if possile.

@codecov
Copy link

codecov bot commented Dec 14, 2021

Codecov Report

Merging #211 (ededcf9) into master (b6aa830) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #211   +/-   ##
=======================================
  Coverage   67.15%   67.15%           
=======================================
  Files          17       17           
  Lines        1629     1629           
=======================================
  Hits         1094     1094           
  Misses        535      535           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6aa830...ededcf9. Read the comment docs.

@eddiebergman eddiebergman requested a review from mfeurer December 15, 2021 05:15
Copy link
Contributor

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

That looks good. I'm wondering whether we now also need to run unit tests etc on Windows. We now know that we can build the wheels, but we don't know whether our packages work there.

@eddiebergman
Copy link
Contributor Author

Click in to the tests, it looks like it runs the unittests on those wheels.

@mfeurer
Copy link
Contributor

mfeurer commented Dec 15, 2021

Thanks a lot for the pointer. This is excellent. I'm wondering whether we should test these other platforms during the regular unit tests as well? But that would be a separate PR I assume?

@eddiebergman
Copy link
Contributor Author

Let's do it as a separate PR, if we run into build issues when doing unit-tests then we might want to have dedicated changes there.

@mfeurer mfeurer merged commit cdad6b3 into automl:master Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants