Use scikit-build-core

[henryiii]

Using scikit-build-core (closes #811).

Ready for review:

• scikit-build-core is not on conda-forge yet (though I might try this patch on it when it does go in).
• Autoconversion from setup.cfg lost a little (maintainer), and was accidentally based on a slightly older commit.
• Editable mode is not supported yet, so that's an expected failure.
• Dynamic versions are not supported yet (though that might be coming soon). [solved by not using dynamic version]

If the current version does any build caching, that's also not supported yet.

[henryiii]

Docs has:

ImportError: cannot import name 'xla_data_pb2' from 'jaxlib' (/opt/hostedtoolcache/Python/3.9.15/x64/lib/python3.9/site-packages/jaxlib/__init__.py)
[69](https://github.com/scikit-hep/iminuit/actions/runs/3493666204/jobs/5848731337#step:8:70)
ImportError: cannot import name 'xla_data_pb2' from 'jaxlib' (/opt/hostedtoolcache/Python/3.9.15/x64/lib/python3.9/site-packages/jaxlib/__init__.py)

Don't think that was me!

Is it possible to trigger the wheel build on this to see if there are any errors there?

[HDembinski]

I also think this is unrelated, I will investigate, but cannot do it on short time scale. Thanks for this.

[HDembinski]

You removed pybind11 as a submodule. I liked it as a submodule because it allows me to use a patched pybind11 if necessary. So far it was not necessary, but I like the flexibility.

[HDembinski]

I looked through the changes, this looks very good. It is nice to get rid of MANIFEST.in, setup.*, and have all setup in pyproject.toml.

I am not sure what the jax issue is, but I haven't run any tests on CI in a while, perhaps just the latest version is buggy.

Using pipx to build the sdist is also neat.

[henryiii]

Next week (just got back from a small vacation) I'll work on the editable install feature. Dynamic versions also probably aren't far away, though I want to think a bit to make sure it's somewhat flexible.

Using pybind11 from the python package is helpful for things like conda-forge, which want you to use the conda-forge package. It also allows us to patch the conda forge package if needed, etc. Though that wasn't too important for this if you want something different, I was mostly making sure it worked (notice it requires no path setup, it's discovered entirely automatically on including it in the pyproject.toml!)

I would highly recommend not patching pybind11, that will increase the maintenance burden. It's very important to get rapid updates from upstream since we are working with the CPython developers every release to adapt to the new CPython API changes and removals (also with PyPy devs). But you could always swap if and when you did need a patch, it's not hard to change to and from a submodule.

[HDembinski]

Next week (just got back from a small vacation) I'll work on the editable install feature. Dynamic versions also probably aren't far away, though I want to think a bit to make sure it's somewhat flexible.

Good point, editable install is very important for my development, so that would be a requirement for me to make the switch.

[HDembinski]

I would highly recommend not patching pybind11, that will increase the maintenance burden. It's very important to get rapid updates from upstream since we are working with the CPython developers every release to adapt to the new CPython API changes and removals (also with PyPy devs). But you could always swap if and when you did need a patch, it's not hard to change to and from a submodule.

Please leave it as a submodule. It is not easy for me to change to and from a submodule, because it affects the builds on conda-forge etc. Patching dependencies locally is obviously a last resort and temporary solution, but I had to do this quite often for ROOT and it is nice to have that flexibility.

[henryiii]

Okay, I can change it next time I edit it - though it's still only a one line change to move to a fork and still take advantage of Python packaging:

-     "pybind11",
+     "pybind11 @ git+https://github.com/my_user/pybind11@my_branch_or_commit"

(I have to do this all the time testing PRs, such as https://github.com/pybind/scikit_build_example/blob/f88b61bd45ea06e0f0e634cb5f5e2f94ea30a1ce/pyproject.toml (neither scikit-build/scikit-build-core nor pybind11 support Windows cross-compiling to ARM currently!) 😆

[HDembinski]

Okay, I can change it next time I edit it

I gave it some more thought and I understand that it is easier for packaging if I use the pybind package instead of the submodule.

So how about this. We leave it as it is now, but you promise to help me should I need to go back in the future to a pybind11 submodule to apply temporarily patch? I am worried that it is more than just this, because of conda-forge integration etc.

[HDembinski]

Dynamic versions are not supported yet (though that might be coming soon).

Oh, I didn't see that. Latest patch is breaking the build now, because I changed back to dynamic version. iminuit single-sources the version from iminuit.version.py. I would prefer to source it from pyproject.toml (seems more natural), but I also need to source the root-version from somewhere, and that's why I use version.py where I can set both. Is it possible to add (and read from Python) arbitrary extra fields to [project] and read those from Python? If the answer is yes, then I could switch to single-sourcing from pyproject.toml.

Different topic: the jax error seems to be related to the fact that the conversion from setup.cfg lost the pinned version for jax and jaxlib.

[HDembinski]

How do I enable parallel compilation? Ok, CMAKE_BUILD_PARALLEL_LEVEL works.

The root_version field [project] is rejected. :( I worked around this by dynamically providing the root version where it is used, in the doc build.

[henryiii]

Different topic: the jax error seems to be related to the fact that the conversion from setup.cfg lost the pinned version for jax and jaxlib.

Not surprised, I accidentally converted from an older checkout, so a few things from setup.cfg were out of date.

arbitrary extra fields

You are allowed to add arbitrary fields to [tool.iminuit], if you wanted to do that. You'd need to add tomli; python_version<3.11 to the deps, etc.

We leave it as it is now, but you promise to help me should I need to go back in the future to a pybind11 submodule to apply temporarily patch?

Sure. Or I can help you get a fix into pybind11 if something like that pops up. ;)

FYI, I've got experimental support for Windows ARM cross-compiles in the latest scikit-build-core release. Though next I'm working on updating cmake/ninja with ARM binaries.

[HDembinski]

• Fixed the tests that "broke" since Jupyter and ipywidgets decided to raise silly deprecation warnings.
• Enabled ccache to accelerate builds
• root_version is not hardcoded anymore, doc build fetches the correct value dynamically
• Fixed the jax issue by pinning the version again

Remaining issue: The editable build is required. Without it, coverage cannot be computed.

[HDembinski]

I am going to cherry pick most of the changes that I made here into #817 and then I will rebase this PR.

[HDembinski]

Calling the branch henryiii/chore/scikit-build-core instead of chore/scikit-build-core is messing with my git.

git branch --track henryiii
warning: refname 'henryiii/henryiii/chore/scikit-build-core' is ambiguous.
fatal: ambiguous object name: 'henryiii/henryiii/chore/scikit-build-core'

[HDembinski]

@henryiii I am learning meson now. I have been hearing good things about meson for a while, and since scipy is moving to meson, it must be about ready to replace other build systems. I saw that you also contributed to the meson-python backend.

As an experiment, I ported pyhepmc to meson. scikit-hep/pyhepmc#57 It does not work yet without editable builds.

Like scikit-build-core, meson-python is lacking editable builds, but the main branch already has some patches for that in place, so it looks like it is coming soon. I might directly switch to meson-build then.

[HDembinski]

Just a little update: meson-python is not feature complete, so I am still interested in scikit-build-core.

meson is nice, but very opinionated about certain points which do not compromise any of their goals. For example, they insist on only supporting single quoted strings in meson.build files, using double quotes for a string is a syntax error. I think they should support both, similar to Python, but the issue was immediately rejected.

[henryiii]

Meson is very opinionated, yes. They will still continue to insist their syntax is "python-like". 😆 I'm funded for three years to work on scikit-build-core, so quite excited for its prospects, and quite a few projects are signed up as well; sadly numpy & scipy had already picked and started on their path by the time the proposal was written.

[henryiii]

For fun, you can try this:

$ pip install -e . --config-settings=editable.rebuild=true
Obtaining file:///Users/henryschreiner/git/scikit-hep/iminuit
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Requirement already satisfied: numpy>=1.21 in ./.venv/lib/python3.11/site-packages (from iminuit==2.21.3) (1.24.2)
Building wheels for collected packages: iminuit
  Building editable for iminuit (pyproject.toml) ... done
  Created wheel for iminuit: filename=iminuit-2.21.3-cp311-cp311-macosx_13_0_x86_64.whl size=304665 sha256=205bdb06cf7aa36e88c44910f11ff3c720c5986b1b808d98e7a28572823bbc97
  Stored in directory: /private/var/folders/_8/xtbws09n017fbzdx9dmgnyyr0000gn/T/pip-ephem-wheel-cache-uizgv9z7/wheels/62/35/41/f343e1b93de090027587867560b61d3e7342041d5d41f4ddfe
Successfully built iminuit
Installing collected packages: iminuit
  Attempting uninstall: iminuit
    Found existing installation: iminuit 2.21.3
    Uninstalling iminuit-2.21.3:
      Successfully uninstalled iminuit-2.21.3
Successfully installed iminuit-2.21.3
$ sed -i "" '24i
$   py::print("Hello from live editable mode");
$ ' src/main.cpp
$ python -c 'import iminuit'
Hello from live editable mode

😄

[HDembinski]

pip install -e . --config-settings=editable.rebuild=true [...]

That is a cool and impressive feature! It goes beyond what I usually need (I rarely need to touch the C++ layer), so it is not a must-have for me, but it is cool.

[HDembinski]

Note: python -c 'import iminuit' initially failed for me with this error

CMake Error at CMakeLists.txt:18 (find_package):
  Could not find a package configuration file provided by "pybind11" with any
  of the following names:

    pybind11Config.cmake
    pybind11-config.cmake

  Add the installation prefix of "pybind11" to CMAKE_PREFIX_PATH or set
  "pybind11_DIR" to a directory containing one of the above files.  If
  "pybind11" provides a separate development package or SDK, be sure it has
  been installed.


make: *** [cmake_check_build_system] Error 1
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/hdembinski/Extern/iminuit/src/iminuit/__init__.py", line 22, in <module>
    from iminuit.minuit import Minuit
  File "/Users/hdembinski/Extern/iminuit/src/iminuit/minuit.py", line 5, in <module>
    from iminuit._core import (
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1002, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 945, in _find_spec
  File "/Users/hdembinski/Extern/iminuit/testme/lib/python3.10/site-packages/_iminuit_editable.py", line 48, in find_spec
    self.rebuild()
  File "/Users/hdembinski/Extern/iminuit/testme/lib/python3.10/site-packages/_iminuit_editable.py", line 84, in rebuild
    result.check_returncode()
  File "/usr/local/Cellar/python@3.10/3.10.9/Frameworks/Python.framework/Versions/3.10/lib/python3.10/subprocess.py", line 457, in check_returncode
    raise CalledProcessError(self.returncode, self.args, self.stdout,

It worked after pip install pybind11 in my virtual environment.

It takes really long to run without producing any output, even though I have ccache installed. I think it would be better to produce verbose output when using this feature. The quiet pip is good for end-users, which do not need to see what's under the hood, but as a developer you generally want to know.

[henryiii]

Yes, you should probably always use this with —-no-build-isolation, otherwise the build environment gets removed. You can use —-config-settings=editable.verbose=true to watch it build. Maybe the default on the verbose should be flipped. I was thinking it can be surprising to have output you were not expecting. python -c “import iminuit; print(iminuit.__version__)” was originally affected (though now it only outputs on stderr, so maybe it’s good enough now to be default verbose).

[eli-schwartz]

Meson is very opinionated, yes. They will still continue to insist their syntax is "python-like". 😆

🤔 I'm actually unable to find a quote for that in the docs, and the closest I can find is:

The main design goals of this language has been simplicity, clarity and conciseness. Much inspiration was drawn from the Python programming language, which is considered very readable, even to people who have not programmed in Python before.

Which isn't very close, really.

Yes, you should probably always use this with —-no-build-isolation, otherwise the build environment gets removed.

People keep hitting this issue in every build backend offering editable installs, seems like. Maybe pip should just disable isolation when editable installs are in use! The current state of affairs seems like really bad UX.

[henryiii]

Maybe pip should just disable isolation when editable installs are in use! The current state of affairs seems like really bad UX.

This would be a bit surprising as well, since it would cause -e to require all needed build backends present in the current environment. Some build backends work from isolation (most (all?) pure Python backends that support editable mode), so the chances of this sort of change being accepted aren't very high, IMO. A single letter shortcut for --no-build-isolation, though, so we could recommend -ne or whatever, would I think be a fair compromise. Given you also almost always want v, then it would be a simple -vne or something like that. And it would be nice if it was a letter we could also adopt on pypa/build. (so not -n, actually!)

[HDembinski]

@henryiii Thank you very much for this work.