Skip to content

BLD: build with numpy instead of oldest-supported-numpy #478

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 1, 2024
Merged

BLD: build with numpy instead of oldest-supported-numpy #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49b8e33
BLD: build with numpy instead of oldest-supported-numpy
neutrinoceros Apr 1, 2024
beedecb
BLD: fix compiling against numpy 2.0
neutrinoceros Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions numexpr/interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,7 @@ NumExpr_run(NumExprObject *self, PyObject *args, PyObject *kwds)
Py_INCREF(dtypes[0]);
} else { // constant, like in '"foo"'
dtypes[0] = PyArray_DescrNewFromType(NPY_STRING);
dtypes[0]->elsize = (int)self->memsizes[1];
PyDataType_SET_ELSIZE(dtypes[0], (npy_intp)self->memsizes[1]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if there is a way to write this so it branches on NPY_ABI_VERSION. That way there could be NumPy 1 & 2 compilation paths

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The easiest way is to define the macro if it isn't defined. Without that users can't do a local build with build isolation on old NumPy (which won't allow to compile with numpy 1 and run in 2, that would be harder).

Clearer instructions: https://numpy.org/devdocs/numpy_2_0_migration_guide.html#the-pyarray-descr-struct-has-been-changed

} // no string temporaries, so no third case
}
if (dtypes[0] == NULL) {
Expand Down Expand Up @@ -1449,7 +1449,7 @@ NumExpr_run(NumExprObject *self, PyObject *args, PyObject *kwds)
/* Get the sizes of all the operands */
dtypes_tmp = NpyIter_GetDescrArray(iter);
for (i = 0; i < n_inputs+1; ++i) {
self->memsizes[i] = dtypes_tmp[i]->elsize;
self->memsizes[i] = PyDataType_ELSIZE(dtypes_tmp[i]);
}
Comment on lines 1449 to 1453
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if it is possible to rewrite this with PyArray_ITEMSIZE as that works on NumPy 1 & 2 based on this release note


/* For small calculations, just use 1 thread */
Expand Down
6 changes: 5 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
[build-system]
requires = ["setuptools", "wheel", "oldest-supported-numpy"]
requires = [
"setuptools",
"wheel",
"numpy>=2.0.0rc1",
]
build-backend = "setuptools.build_meta"
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
numpy >= 1.13.3
numpy >= 1.19.3 # keep in sync with NPY_TARGET_VERSION (setup.py)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly the version constraint here could be dropped if we just want to support the oldest NumPy available on Python 3.9. Unless of course there are other reasons than NumPy C API to set a NumPy minimum

Suggested change
numpy >= 1.19.3 # keep in sync with NPY_TARGET_VERSION (setup.py)
numpy

5 changes: 4 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
inc_dirs = [np.get_include()]
libs = [] # Pre-built libraries ONLY, like python36.so
clibs = []
def_macros = []
def_macros = [
# keep in sync with minimal runtime requirement (requirements.txt)
('NPY_TARGET_VERSION', 'NPY_1_19_API_VERSION')
]
Comment on lines +36 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIUI this shouldn't be needed unless numexpr is doing something more particular with the NumPy API

During the conda-forge NumPy 2 bringup discussion we talked to @rgommers about this, Ralf wrote up a nice explanation here ( conda-forge/conda-forge.github.io#1997 (comment) ). Quoting that below:

  • numpy's pyproject.toml contains and enforces the lowest-supported version, e.g.: requires-python = ">=3.9"
  • the first numpy version to support that python version (e.g., 1.19.0 for py39) determined the maximum version that NPY_FEATURE_VERSION may be set to
  • when that >=3.9 is bumped to >=3.10, then that max-allowed version for NPY_FEATURE_VERSION moves up.

So would suggest dropping

Suggested change
def_macros = [
# keep in sync with minimal runtime requirement (requirements.txt)
('NPY_TARGET_VERSION', 'NPY_1_19_API_VERSION')
]
def_macros = []

sources = ['numexpr/interpreter.cpp',
'numexpr/module.cpp',
'numexpr/numexpr_object.cpp']
Expand Down