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.

Sign up for GitHub

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

Jump to bottom

python3.pkgs.scikitlearn: 0.20.3 -> 0.21.2 #63591

Merged
merged 1 commit into from Jun 22, 2019
Merged

python3.pkgs.scikitlearn: 0.20.3 -> 0.21.2 #63591

merged 1 commit into from Jun 22, 2019

Conversation

timokau
Copy link
Member

@timokau timokau commented Jun 20, 2019 •
edited

Motivation for this change

Splits scikitlearn into a python2 (0.20.x) and a python3 (>0.20)
version.

Currently blocked by #63573.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@timokau timokau requested a review from FRidh as a code owner June 20, 2019 21:28
@risicle
Copy link
Contributor

risicle commented Jun 22, 2019

In my basic testing, the bump to 0.21.2 fixes #63573

@risicle
Copy link
Contributor

risicle commented Jun 22, 2019 •
edited

This fails on darwin (tested macos 10.13) due to lack of omp.h, to fix, add to buildInputs: ++ stdenv.lib.optional stdenv.cc.isClang llvmPackages.openmp ...or something like that...

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

In my basic testing, the bump to 0.21.2 fixes #63573

So you can build and test python3.pkgs.scikitlearn from the branch of this PR without errors? Because I can't.

@risicle
Copy link
Contributor

risicle commented Jun 22, 2019

I can. Non-nixos linux x86_64.

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

That is weird. Sandboxed?

Can you double check that you're building from this PR (commit 7d43a86c2fbd739658b1b6af5414b34d8a347a47) and are building the python3 version?

@risicle
Copy link
Contributor

risicle commented Jun 22, 2019

Sandboxed. Commit 7d43a86c2fbd739658b1b6af5414b34d8a347a47.

Resultant package is /nix/store/gn4il29vjm5h2kgjm5z31m9wzk6h2lb7-python3.7-scikit-learn-0.21.2.

@risicle
Copy link
Contributor

risicle commented Jun 22, 2019

As I say, it also passes on my macos 10.13 machine if I give it llvmPackages.openmp.

@timokau
python3.pkgs.scikitlearn: 0.20.3 -> 0.21.2
7035680 
Splits scikitlearn into a python2 (0.20.x) and a python3 (>0.20)
version.
@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

As I say, it also passes on my macos 10.13 machine if I give it llvmPackages.openmp.

Oh, I thought I had already pushed that. Now I have.

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

Weird. I can also successfully build it on two different nixos machines. On the machine that fails, I do actually have /nix/store/gn4il29vjm5h2kgjm5z31m9wzk6h2lb7-python3.7-scikit-learn-0.21.2 in the store already. It tries and fails to build python3.pkgs.scikitlearn anyways.

@timokau timokau changed the title [WIP] python3.pkgs.scikitlearn: 0.20.3 -> 0.21.2 python3.pkgs.scikitlearn: 0.20.3 -> 0.21.2 Jun 22, 2019
@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

Either way, since the build is currently broken anyway it can't make things worse.

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

Let's see what ofBorg has to say.

@GrahamcOfBorg build python2.pkgs.scikitlearn python3.pkgs.scikitlearn

@risicle
Copy link
Contributor

risicle commented Jun 22, 2019

I assume it's the accuracy-related test failing for you?

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

Yep

@timokau
Copy link
Member Author

timokau commented Jun 22, 2019

Well as I said, at least it doesn't make it any worse and it does improve things for most people.

@timokau timokau merged commit 78054a5 into NixOS:master Jun 22, 2019
@timokau timokau deleted the scikitlearn-0.21 branch June 22, 2019 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@risicle risicle risicle left review comments

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@timokau @risicle