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

python3Packages.scipy: disable failing tests on aarch64-darwin #253531

Merged
merged 2 commits into from
Sep 8, 2023
Merged

python3Packages.scipy: disable failing tests on aarch64-darwin #253531

merged 2 commits into from
Sep 8, 2023

Conversation

reckenrode
Copy link
Contributor

Description of changes

Some tests fail on aarch64-darwin when SciPy is built with a newer
compiler (such as clang 16). This is not yet fixed upstream, so disable
them until they work again.

See scipy/scipy#18308

Testing was delegated to the scipy test suite, which ran on all supported platforms after the change.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@reckenrode reckenrode mentioned this pull request Sep 5, 2023
Closed
@reckenrode reckenrode changed the base branch from master to staging September 5, 2023 22:15
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Sep 5, 2023
@ofborg ofborg bot requested review from FRidh and doronbehar September 5, 2023 22:54
doronbehar
doronbehar reviewed Sep 6, 2023
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Looks great overall! Thanks for the work.

doronbehar
doronbehar reviewed Sep 7, 2023
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Almost perfect!

pkgs/development/python-modules/scipy/default.nix Outdated Show resolved Hide resolved
reckenrode and others added 2 commits September 7, 2023 08:25
@reckenrode @doronbehar
python3Packages.scipy: support for disabling tests
7fc80bc 
Adapt the logic from pytestCheckHook to the custom check phase used by
scipy.

Co-authored-by: Doron Behar <[email protected]>
@reckenrode
python3Packages.scipy: disable failing tests on aarch64-darwin
422ef04 
Some tests fail on aarch64-darwin when SciPy is built with a newer
compiler (such as clang 16). This is not yet fixed upstream, so disable
them until they work again.

See scipy/scipy#18308
@doronbehar
Copy link
Contributor

Great, and have you tested this on your Darwin machine?

@doronbehar
Copy link
Contributor

I mean, with the multiline python command we changed there.

@ofborg ofborg bot requested a review from doronbehar September 7, 2023 13:44
@reckenrode
Copy link
Contributor Author

Great, and have you tested this on your Darwin machine?
I mean, with the multiline python command we changed there.

I tested before I did the force-push. It still builds on aarch64-darwin with clang 16 and the tests enabled.

@doronbehar
Copy link
Contributor

OK I tested your changes as well and they seem fine - I enabled the disabledTests attribute for all platforms to make sure the scipy.test command works with it as well.

However, Reading the code once more made me think that in general (both here and in pytest-check-hook.sh we don't need that Bash code to generate the -k ... arguments, but we can do that with Nix... That's out of scope for this PR however.

@doronbehar doronbehar merged commit b25e9e5 into NixOS:staging Sep 8, 2023
@SuperSandro2000
Copy link
Member

I just wanted to chime in and say thanks ❤️ because this is just what I needed right now. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh Awaiting requested review from FRidh

@doronbehar doronbehar Awaiting requested review from doronbehar

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: python 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@reckenrode @doronbehar @SuperSandro2000