Skip to content

Conversation

@stefanv
Copy link
Member

@stefanv stefanv commented Mar 19, 2025

TODO:

  • Add test

@stefanv stefanv mentioned this pull request Mar 19, 2025
18 tasks
@stefanv stefanv added the type: Enhancement New feature or request label Mar 19, 2025
@czgdp1807
Copy link
Contributor

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.

Copy link
Contributor

@czgdp1807 czgdp1807 left a 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)

@czgdp1807
Copy link
Contributor

@stefanv Is this PR good to go? It worked well for me.

cc: @rgommers

@ev-br
Copy link
Contributor

ev-br commented Apr 1, 2025

Just to check my understanding. I've a use case now where I do in SciPy

$ python dev.py build -C-Dblas=mkl    # invoke a meson option
$ python dev.py test -t scipy/linalg/tests -v     # must not rebuild

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?

@rgommers
Copy link
Contributor

rgommers commented Apr 1, 2025

@ev-br I think the second command doesn't reconfigure the build and hence it still uses MKL. E.g., try python dev.py build -Cnonsense=true - I don't think it will choke. Hence you shouldn't need this PR.

Copy link
Contributor

@rgommers rgommers left a 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.

@stefanv
Copy link
Member Author

stefanv commented Apr 9, 2025

I'm happy to reduce the option to just --no-build. I'm a bit hesitant to add one-letter shortcuts. -n, e.g., is commonly used to mean dry-run. Because this option is global, it overrides every command out there. Would --no-build suffice for now?

@rgommers
Copy link
Contributor

rgommers commented Apr 9, 2025

Would --no-build suffice for now?

Sure, that works too, and seems safe. If it starts getting used heavily we can always invent a one-letter acronym in the future.

@stefanv
Copy link
Member Author

stefanv commented Apr 9, 2025

OK, removed the --build flag, the rest of behavior remains the same. If tests pass, @jarrodmillman this should be good to go.

@czgdp1807
Copy link
Contributor

czgdp1807 commented Apr 22, 2025

@stefanv Let's merge this PR. :-) The tests are passing.

@stefanv stefanv merged commit 164d4a5 into scientific-python:main Apr 22, 2025
17 checks passed
@stefanv
Copy link
Member Author

stefanv commented Apr 22, 2025

OK :)

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

Labels

type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants