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

python-modules.opt-einsum: Fix broken build since upgrading numpy #93107

Closed
wants to merge 1 commit into from
Closed

python-modules.opt-einsum: Fix broken build since upgrading numpy #93107

wants to merge 1 commit into from

Conversation

avdv
Copy link
Member

@avdv avdv commented Jul 14, 2020

Motivation for this change

The build for opt-einsum was broken since 26. June, probably because of upgrading numpy.

https://hydra.nixos.org/build/123886782

This fixes the apparent problem here, but I'll try to come up with a patch in the upstream project.

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.

@avdv
python-modules.opt-einsum: Fix broken build since upgrading numpy
d5adacf 
The test was broken since upgrading the numpy module because they
changed the exception type from `TypeError` to `ValueError`.
@ofborg ofborg bot requested a review from teh July 14, 2020 12:16
@avdv avdv mentioned this pull request Jul 15, 2020
Closed
10 tasks
milibopp
milibopp approved these changes Jul 15, 2020
View reviewed changes
Copy link
Contributor

@milibopp milibopp left a comment

Choose a reason for hiding this comment

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

Reviewed points
  • package build on x86_64
  • all depending packages build: there are broken dependents, but not due to this PR.
Comments

Adding a patch is somewhat brittle and should definitely be fixed upstream asap. However, it only affects the test suite. And it is still better than the package not building at all.

teh
teh approved these changes Jul 18, 2020
View reviewed changes
Copy link
Contributor

@teh teh left a comment

Choose a reason for hiding this comment

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

Thanks!

@avdv
Copy link
Member Author

avdv commented Jul 23, 2020

This PR is now obsolete, since there is a new upstream release which fixes the problem.

@jonringer
Copy link
Contributor

no longer needed due to #93907

@jonringer jonringer closed this Jul 31, 2020
@jonringer
Copy link
Contributor

@avdv thanks for opening the PR :)

@avdv avdv deleted the fix-opt-einsum branch September 15, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teh teh teh approved these changes

@milibopp milibopp milibopp approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

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
4 participants
@avdv @jonringer @teh @milibopp