BLD: use install-tags to optionally install tests

[czgdp1807]

#25737

[rgommers]

The failing CI jobs show #include "_numpyconfig.h" missing. The fix should be to include devel in the set of default tags to install (that consists of the headers and the two static libraries we ship).

[rgommers]

The next step here is to extend tools/check_installed_files.py to add a flag ensuring none rather than all tests are installed, right?

[rgommers]

A few more things to mark as tests or move to the right place:

• numpy/_pyinstaller/test_pyinstaller.py should be moved to numpy/_pyinstaller/tests/test_pyinstaller.py to treat it correctly
• Some test extension modules in numpy/_core/meson.build should be marked with the tests tag: _umath_tests, _struct_ufunc_tests, _multiarray_tests, _operand_flag_tests, rational_tests.

[czgdp1807]

Let me test on CI before shifting files to correct places.

[czgdp1807]

The next step here is to extend tools/check_installed_files.py to add a flag ensuring none rather than all tests are installed, right?

Now this will be done in next commit.

[czgdp1807]

(numpy-dev) 16:25:13:~/Quansight/numpy % python tools/check_installed_files.py build-install/usr/lib/python3.11/site-packages/numpy --no-tests
----------- No test files were installed --------------
----------- All the necessary .pyi files were installed --------------

With runtime and python-runtime tags locally. I think we can add one linux job on CI which only uses runtime and python-runtime tags. Then check using check_installed_files.py. @rgommers What do you say?

[rgommers]

With runtime and python-runtime tags locally. I think we can add one linux job on CI which only uses runtime and python-runtime tags. Then check using check_installed_files.py. @rgommers What do you say?

Also add devel I think. Other than that, yes. You can make this really cheap by not doing it in a new job, but in the existing one which already uses python check_installation.py I think. Something like:

rm -rf build-install
./vendored-meson/meson/meson.py install -C build --destdir ../build-install --tags=runtime,python-runtime,devel
python check_installation.py build-install

[czgdp1807]

Sure. I also wanted to do something similar but a separate job came into my mind to avoid pollution and doing rm -rf build_install.

[rgommers]

This is starting to look good. I noticed a single issue that breaks an installed numpy without tests:

$ python -c "import numpy as np"
Traceback (most recent call last):
  File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.11/site-packages/numpy/__init__.py", line 114, in <module>
    from numpy.__config__ import show as show_config
  File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.11/site-packages/numpy/__config__.py", line 4, in <module>
    from numpy._core._multiarray_umath import (
  File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.11/site-packages/numpy/_core/__init__.py", line 96, in <module>
    from . import _add_newdocs
  File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.11/site-packages/numpy/_core/_add_newdocs.py", line 4874, in <module>
    add_newdoc('numpy._core._multiarray_tests', 'format_float_OSprintf_g',
  File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.11/site-packages/numpy/_core/function_base.py", line 549, in add_newdoc
    new = getattr(__import__(place, globals(), {}, [obj]), obj)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'numpy._core._multiarray_tests'

That private test function doesn't really need a docstring; it isn't rendered anywhere. You could move the content from _add_newdocs.py to a code comment for format_float_OSprintf_g in numpy/_core/src/multiarray/_multiarray_tests.c.src.

Aside from that, everything works for me when testing against SciPy. And the numpy wheel changes by ~1.6 MB; on a local wheel build on Linux it goes from 7.6 MB to 6.0 MB (EDIT: and on macOS from 5.1 MB to 3.6 MB).

a separate job came into my mind to avoid pollution and doing rm -rf build_install.

This is the kind of thing where we have to be pragmatic. A separate job would be ~10 minutes, while a single step at the end of the existing job takes only a few seconds, and can be implemented with less code for the CI test. The rm -rf isn't really an issue.

[czgdp1807]

This is the kind of thing where we have to be pragmatic.

Yes correct.

The rm -rf isn't really an issue.

It's just that sometimes previous commands influence the results of later commands, especially in case of builds. However I think rm -rf build_install should work fine. Personally I prefer to do, git clean -fdx (just to completely clear the output of previous commands) and make sure the tests actually do what we expect. I am fine with rm -rf build_install as well.

[rgommers]

One more idea for a check to make things more robust. We have only four tags, and we should ensure that everything indeed does have one of those tags to avoid some file going silently missing (if it doesn't have a tag, it will no longer be installed). Something like this should do it:

import json
import os

all_tags = set()

with open(os.path.join('build', 'meson-info', 'intro-install_plan.json'), 'r') as f:
    targets = json.load(f)

for key in targets.keys():
    for values in list(targets[key].values()):
         if not values['tag'] in all_tags:
            all_tags.add(values['tag'])

if not all_tags == set(['runtime', 'runtime-python', 'devel', 'tests']):
    raise AssertionError(f"Found unexpected install tag: {all_tags}")

This could go in the same check_installation.py file I think. It's always a useful check, so it can be done unconditionally.

[rgommers]

After that I think this is about ready to merge. Docs and actually changing the wheel contents is probably best done in a follow-up PR.

[czgdp1807]

I think meson should do that? I mean it meson's tags should be checked by it in the build step. If some unexpected tag is found then meson should raise error, instead of us running our own check_installed_files.py. Wouldn't it be redundant?

[rgommers]

meson will emit a warning that is visible in the meson-logs/meson-log.txt file, but it won't error and there is no flag to make it error. Build warnings are quite easily missed in PR reviews, no one really looks at that log file until something breaks. So I think it's worth doing.

[czgdp1807]

Ah! Is warning a right response for unexpected tags? I think meson should raise an error and abort the build procedure if any unexpected/undeclared tag is encountered. A warning is silent response. We can do it on our own then as you suggested but I think this error raising should be done by meson itself. I am not aware of the reasoning behind giving a warning instead of en error. I am interested to know though.

[rgommers]

I am not aware of the reasoning behind giving a warning instead of en error. I am interested to know though.

The reason is two-fold I think:

1. Not everything can be automatically assigned a tag. Many things are, like .h files get 'devel', .py files get 'python-runtime', etc. But if you have an unknown file extension or the output of say a custom_target, then Meson cannot know what the right tag is.
2. Backwards compatibility. install tags were introduced fairly recently, and they are an optional feature. Many projects don't use them, and hence don't take the trouble to explicitly tag things. Meson starting to raise an error would therefore break all those projects' builds.

[czgdp1807]

I guess one CI job failing is irrelevant? May be happened due to I added [skip azp] [skip cirrus] [skip circle] in my last commit.

[rgommers]

I guess one CI job failing is irrelevant? May be happened due to I added [skip azp] [skip cirrus] [skip circle] in my last commit.

Not due to that, it's just a random unrelated failure due to an issue with musllinux scipy-openblas32 wheel somehow. You can simply ignore it.

[czgdp1807]

All set on this PR from my end.

[rgommers]

Thanks @czgdp1807. I pushed two small fixes. Code comment style as prescribed in https://numpy.org/neps/nep-0045-c_style_guide.html for multi-line comments, and a fix for the pyinstaller test because I had a hunch that we weren't running that in CI anymore (it's super slow, 40 seconds on my machine).

If CI is happy, I think this is about ready.

[czgdp1807]

Tests pass. :D.

[rgommers]

This PR shouldn't change anything yet, it only makes it possible to omit installing the tests. The next step is to actually start doing that - I opened gh-26289 with a few of the decisions that need making.

[rgommers]

Merged - thanks again @czgdp1807!