-
-
Notifications
You must be signed in to change notification settings - Fork 28
Use uv
and simplify wheel building
#234
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
efa2e14
to
7c919b3
Compare
fa152f0
to
b3336aa
Compare
cysignals is a Python package - why would it need to ship a .pc file? To help cython headers discovery? |
without
|
we can also do the numpy-style way, adding a diff --git a/example/meson.build b/example/meson.build
index 04cc3ef..945e9eb 100644
--- a/example/meson.build
+++ b/example/meson.build
@@ -2,12 +2,22 @@ project('cysignals_example', 'cython', 'cpp')
py = import('python').find_installation()
-cysignals = dependency('cysignals')
+incdir_cysignals = run_command(py,
+ ['-c',
+ '''import os
+import cysignals as csi
+print(csi.get_include())
+ '''
+ ],
+ check : true
+).stdout().strip()
+
+inc_cysignals = include_directories(incdir_cysignals)
py.extension_module('cysignals_example',
sources: ['cysignals_example.pyx'],
install: true,
- dependencies: [cysignals],
+ include_directories: inc_cysignals,
override_options: ['cython_language=cpp'],
subdir: 'cysignals_example'
)
diff --git a/example/pyproject.toml b/example/pyproject.toml
index eb839e6..438c758 100644
--- a/example/pyproject.toml
+++ b/example/pyproject.toml
@@ -1,5 +1,5 @@
[build-system]
-requires = ["meson-python", "cython>=0.28", "pkgconf"]
+requires = ["meson-python", "cython>=0.28", "cysignals"]
build-backend = "mesonpy"
[project]
diff --git a/src/cysignals/__init__.py b/src/cysignals/__init__.py
index 81efe32..931243e 100644
--- a/src/cysignals/__init__.py
+++ b/src/cysignals/__init__.py
@@ -1,3 +1,7 @@
from .signals import AlarmInterrupt, SignalError, init_cysignals # noqa
init_cysignals()
+
+def get_include():
+ import os
+ return os.path.dirname(__file__)
diff --git a/src/cysignals/cysignals.pc b/src/cysignals/cysignals.pc
deleted file mode 100644
index d14a6b8..0000000
--- a/src/cysignals/cysignals.pc
+++ /dev/null
@@ -1,6 +0,0 @@
-includedir=${pcfiledir}
-
-Name: cysignals
-Description: cysignals library
-Version: 1.0.0
-Cflags: -I${includedir}
diff --git a/src/cysignals/meson.build b/src/cysignals/meson.build
index e9aa9eb..062ebfe 100644
--- a/src/cysignals/meson.build
+++ b/src/cysignals/meson.build
@@ -2,7 +2,6 @@ configure_file(output: 'cysignals_config.h', configuration: config, install_dir:
py.install_sources(
'__init__.py',
- 'cysignals.pc',
'cysignals-CSI-helper.py',
'memory.pxd',
'pysignals.pxd', It's a bit more verbose, but more conformant to what is around. As far as the example is concerned, it's a bit meaningless. its README says that it's tested, but I fail to see any trace of it. |
Yes, to make it easier to be used downstream (e.g in sage). For cysignals it's indeed only the headers, but cypari2 should also specify where pari is located. Numpy is also shipping a pc file for the same reason: https://github.com/numpy/numpy/blob/main/numpy/_core/numpy.pc.in. The
Yes, to make it with build isolation one needs to properly setup a uv workspace so that the example is correctly using the cysignals in the root and not from pypi. I don't think the example is super important... It's build is tested though ( Line 114 in 896256e
meson test (which is run by CI).
|
Could you explain how to invoke Line 114 in 896256e
I am getting, by looking what CI does:
|
I also had that issue sometimes, but it disappeared after deleting the venv. The following commands should work on a clean checkout:
It's a bit more complicated than I would like, but uv doesn't have good support for |
right, it seems to be caused by an external to the venv installation of cysignals. Can you put these 3 lines somewhere in the docs?
|
Thanks for the review, and for adding the instructions. What do you think about issuing a new release so that those new shiny wheels are used? |
good idea - past 23:00 here, tomorrow? |
@tobiasdiez - what's the release procedure in the presence of |
I wonder whether one should also need to do something with the package versions locked in |
|
uv
instead of the old-stylerequirements.txt
cysignals = dependency('cysignals')
.