Skip to content

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

Merged
doronbehar merged 2 commits intoNixOS:stagingfrom
reckenrode:scipy-fix
Sep 8, 2023
Merged

python3Packages.scipy: disable failing tests on aarch64-darwin #253531
doronbehar merged 2 commits intoNixOS:stagingfrom
reckenrode:scipy-fix

Conversation

@reckenrode
Copy link
Copy Markdown
Contributor

@reckenrode reckenrode commented Sep 5, 2023

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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Sep 5, 2023
@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
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Sep 5, 2023
doronbehar
doronbehar reviewed Sep 6, 2023
View reviewed changes
doronbehar
doronbehar reviewed Sep 6, 2023
View reviewed changes
Copy link
Copy Markdown
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
doronbehar
doronbehar reviewed Sep 7, 2023
View reviewed changes
Copy link
Copy Markdown
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!

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 <doron.behar@gmail.com>
@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
Copy Markdown
Contributor

doronbehar commented Sep 7, 2023

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

@doronbehar
Copy link
Copy Markdown
Contributor

doronbehar commented Sep 7, 2023

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
Copy Markdown
Contributor Author

reckenrode commented Sep 7, 2023

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
Copy Markdown
Contributor

doronbehar commented Sep 8, 2023

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
Copy Markdown
Member

SuperSandro2000 commented Sep 28, 2023

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 Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@reckenrode @doronbehar @SuperSandro2000