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

pythonPackages.datatable: 0.10.1 -> 0.11.0, fix build #98688

Merged
merged 1 commit into from Sep 27, 2020

Conversation

risicle
Copy link
Contributor

@risicle risicle commented Sep 24, 2020

Motivation for this change

ZHF: #97479

I thought giving datatable a bump might have been a short cut to getting it building again - how wrong was I? It looks like it's switched to a new build system and quite a few tweaks needed to be made.

Once we've made some alterations to the source (which doesn't seem to have been shipped in a ready-to-build state), we can use the funny custom build system to build a wheel which can then be installed by the pipInstallHook.

Running the tests is again annoying because an ini file setting pytest's python_files parameter is missing, meaning we have to inject our own.

Finally I had to fix it up for macos, which luckily only involved adding a manual header path, but meant I could get rid of a bunch of patchery and other cruft.

Have not tested all subsequent rebuilds, but it's late. Now I have and the only failure I get is python37Packages.optuna on x86_64 linux (which seems to be a keras/tensorflow issue?)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@drewrisinger drewrisinger left a comment

Choose a reason for hiding this comment

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

  • Diff LGTM minus potentially using pytestCheckHook
  • Commits LGTM
  • Build in progress, only failure so far is python37Packages.cma which is broken on master

pkgs/development/python-modules/datatable/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/datatable/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/datatable/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/datatable/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/datatable/default.nix Outdated Show resolved Hide resolved
@risicle risicle force-pushed the ris-datatable-0.11.0 branch 2 times, most recently from 2058dda to 4578cab Compare September 27, 2020 16:15
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved
once we've made some alterations to the source (which doesn't seem to have
been shipped in a ready-to-build state), we can use the funny custom build
system to build a wheel which can then be installed by the pipInstallHook.

running the tests is again annoying because an ini file setting pytest's
python_files parameter is missing, meaning we have to inject our own.
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

failures are broken on target branch

https://github.com/NixOS/nixpkgs/pull/98688
1 package failed to build:
python37Packages.optuna

6 packages built:
python37Packages.dask-xgboost python37Packages.datatable python37Packages.xgboost python38Packages.dask-xgboost python38Packages.datatable python38Packages.xgboost

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.

None yet

3 participants