Is this code tested? Maybe we should be moving the f2py test builds to meson

Is this code tested? Maybe we should be moving the f2py test builds to meson

It should be tested with meson on the 3.12 builds AFAIK EDIT: I can't remember making changes to the testsuite to work with meson-f2py so it is likely skipped right now, will fix the tests this week(end).

Is this code tested? Maybe we should be moving the f2py test builds to meson

It should be tested with meson on the 3.12 builds AFAIK EDIT: I can't remember making changes to the testsuite to work with meson-f2py so it is likely skipped right now, will fix the tests this week(end).

@mattip a small update, this PR is "sort of" tested in #25111. "sort of" because since #9673 back in 2017 none of the (compiled) F2PY tests are actually run on Windows. However I did test them manually, with a compatible toolchain, something which is hard to guarantee on the CI without distutils anyway. I think, since @jmrohwer also confirmed this is required on Windows it should go in (along with #25114) so we can merge #25111 and restore the pre-meson status quo.

I don't understand. We should be testing this. #9673 is a still-open issue. We have the rtools toolchain on windows with a fortran compiler specifically for tests.

I don't understand. We should be testing this. #9673 is a still-open issue. We have the rtools toolchain on windows with a fortran compiler specifically for tests.

There are two problems here, firstly that (as noted) the F2PY tests have long been skipping all compiled Fortran module tests, git blame shows that this has been around for 6 years now. So Windows (with or without rtools) is not being tested for the compiled f2py modules in NumPy. This was true even before the meson transition.

^--- This in itself isn't really an issue, I believe #9673 should probably be defunct with meson anyway, but there's a larger issue.

Compiler detection

Offloading the build step to meson means the only control knobs are environment variables. Currently:

• f2py, we call meson via subprocess.run when -c is requested

So what needs to happen is that we would need to propagate CLI arguments as environment variables through to all the subprocess.run calls (asv does this). This would be pretty much what was being done before with distutils, and forces us to keep adapting to the environment variables accepted by meson. The current expectation is that users will generate the binding code and customize the build command.

Now even assuming we should modify the frontend to pass environment variables over to meson there's another problem, that of compiler detection. meson doesn't explictly have a Python API, so that means that to figure out what compiler is present on the CI, we'd need to parse the output of meson setup and track that state throughout the code. This is essentially distutils but with meson inside.

To me, it seems like it would dampen much of the benefits we gained (offloading the build complexity, not tracking compilers etc.) from the switch to meson for adding tests on a platform which wasn't really tested to begin with. Also pinging @rgommers for thoughts regarding the meson part.

CI Testing

This doesn't mean it is impossible to test on CI, just that the current interface is too rigid for it, and that the code shouldn't be bent out of shape just to satisfy the unit testing requirement.

Instead, I believe the solution is to have a new CI job, where (some) f2py tests should be tested the way users would use them, i.e. by generating the meson build directory with the skeleton template and then running meson within with the right environment variables.

Perhaps this should be moved to an issue of its own?

that of compiler detection. meson doesn't explictly have a Python API, so that means that to figure out what compiler is present

We know what compiler is present on CI, we don't have to guess.

a new CI job, where (some) f2py tests should be tested the way users would use them, i.e. by generating the meson build directory with the skeleton template and then running meson within with the right environment variables.

I opened #25134

Thanks @HaoZeke

Offloading the build step to meson means the only control knobs are environment variables.

Not quite, you can supply a machine file to point at any binary: https://mesonbuild.com/Machine-files.html#binaries. This is for example what meson-python does to select the Python interpreter to use, it passes it via --native-file.

@HaoZeke I see that this has been merged and #25000 has been closed. So not sure what the right place is to mention this.

However, for f2py -c to work properly on Windows, the built binaries need to be moved to the build root as well (currently only *.so files are searched and moved).

The following patch fixes it:

diff --git a/numpy/f2py/_backends/_meson.py b/numpy/f2py/_backends/_meson.py
index 3176a5e08..14f9ace2b 100644
--- a/numpy/f2py/_backends/_meson.py
+++ b/numpy/f2py/_backends/_meson.py
@@ -7,6 +7,7 @@
 
 from ._backend import Backend
 from string import Template
+from itertools import chain
 
 import warnings
 
@@ -84,6 +85,7 @@ def __init__(self, *args, **kwargs):
     def _move_exec_to_root(self, build_dir: Path):
         walk_dir = Path(build_dir) / self.meson_build_dir
         path_objects = walk_dir.glob(f"{self.modulename}*.so")
+        path_objects = chain(path_objects, walk_dir.glob(f"{self.modulename}*.pyd"))
         for path_object in path_objects:
             shutil.move(path_object, Path.cwd())

Should I open a new bug report or a pull request (seems silly for 2 lines of code)?

@HaoZeke I see that this has been merged and #25000 has been closed. So not sure what the right place is to mention this.

However, for f2py -c to work properly on Windows, the built binaries need to be moved to the build root as well (currently only *.so files are searched and moved).

The following patch fixes it:

diff --git a/numpy/f2py/_backends/_meson.py b/numpy/f2py/_backends/_meson.py
index 3176a5e08..14f9ace2b 100644
--- a/numpy/f2py/_backends/_meson.py
+++ b/numpy/f2py/_backends/_meson.py
@@ -7,6 +7,7 @@
 
 from ._backend import Backend
 from string import Template
+from itertools import chain
 
 import warnings
 
@@ -84,6 +85,7 @@ def __init__(self, *args, **kwargs):
     def _move_exec_to_root(self, build_dir: Path):
         walk_dir = Path(build_dir) / self.meson_build_dir
         path_objects = walk_dir.glob(f"{self.modulename}*.so")
+        path_objects = chain(path_objects, walk_dir.glob(f"{self.modulename}*.pyd"))
         for path_object in path_objects:
             shutil.move(path_object, Path.cwd())

Should I open a new bug report or a pull request (seems silly for 2 lines of code)?

Actually a pull request would be great (we never have any patches committed to master) :)