-
Notifications
You must be signed in to change notification settings - Fork 25
Add ability to switch pre-build on/off #272
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
Conversation
|
Seems to be working for me, (scipy-dev) 23:53:12:~/Quansight/scipy % spin test --no-build
$ export PYTHONPATH="/Users/czgdp1807/Quansight/scipy/build-install/usr/lib/python3.12/site-packages:/Users/czgdp1807/Quansight/scipy/build-install/usr/lib/python3.12/site-packages:"
$ /Users/czgdp1807/mambaforge/envs/scipy-dev/bin/python3.12 -P -c 'import scipy'
$ cd /Users/czgdp1807/Quansight/scipy/build-install/usr/lib/python3.12/site-packages
$ /Users/czgdp1807/mambaforge/envs/scipy-dev/bin/python3.12 -P -m pytest -m 'not slow' --pyargs scipy --pyargs scipy
===================================================== test session starts ======================================================
platform darwin -- Python 3.12.0, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/czgdp1807/Quansight/scipy
configfile: pytest.ini
plugins: cov-6.0.0, hypothesis-6.121.1, timeout-2.3.1, anyio-4.6.2.post1, xdist-3.6.1
collecting 46905 items ^collected 48031 items
Aborted!
(scipy-dev) 23:53:53:~/Quansight/scipy % spin test
Invoking `build` prior to running tests:
$ meson compile -j 10 -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /Users/czgdp1807/mambaforge/envs/scipy-dev/bin/ninja -C /Users/czgdp1807/Quansight/scipy/build -j 10
ninja: Entering directory `/Users/czgdp1807/Quansight/scipy/build'
[2/338] Generating subprojects/highs/src/HConfig.h with a custom command
$ meson install --only-changed -C build --destdir ../build-install --tags=runtime,python-runtime,tests,devel
^C
Aborted!So I am approving. |
czgdp1807
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with reference to #272 (comment)
|
Just to check my understanding. I've a use case now where I do in SciPy So is this a use case this PR aims to address? If so, I'd be willing to test it. What do I need then -- this branch of spin + Gagan's SciPy PR branch? |
|
@ev-br I think the second command doesn't reconfigure the build and hence it still uses MKL. E.g., try |
rgommers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works as advertised for me on numpy. The changes largely look good. Seems fine to merge as is, although it may be useful to make one change: I'd choose -n, --no-build as an option with a one-letter alias, just like -c, -v and -C. Adding --build as a default doesn't really do anything, and I don't see why anyone would use that.
|
I'm happy to reduce the option to just |
Sure, that works too, and seems safe. If it starts getting used heavily we can always invent a one-letter acronym in the future. |
|
OK, removed the |
|
@stefanv Let's merge this PR. :-) The tests are passing. |
|
OK :) |
TODO: