Skip to content
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

Python bindings: add a pyproject.toml with numpy as a build requirement #8926

Merged
merged 9 commits into from
Dec 14, 2023
Merged
1 change: 1 addition & 0 deletions .github/workflows/cmake_builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ jobs:
env:
architecture: x64
generator: Visual Studio 17 2022
GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY: YES
steps:
# To avoid git clone to mess with the line endings of GDAL autotest data
# files that look like text, but should be handled as binary content
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
From c8e8942b5dd50d1abb4369662b0cb9d282c3bf69 Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Wed, 6 Dec 2023 19:06:08 +0100
Subject: [PATCH] Fix build of Python bindings due to
https://github.com/OSGeo/gdal/pull/8926 changes

---
recipe/install_python.sh | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/recipe/install_python.sh b/recipe/install_python.sh
index 4bcd219..1561771 100644
--- a/recipe/install_python.sh
+++ b/recipe/install_python.sh
@@ -21,12 +21,14 @@ cmake "-UPython*" \
cmake --build . --target python_generated_files
cd swig/python

-cat >pyproject.toml <<EOF
-[build-system]
-requires = ["setuptools>=40.8.0", "wheel"]
-build-backend = "setuptools.build_meta"
-EOF
-
$PYTHON setup.py build_ext

+# Cf https://github.com/OSGeo/gdal/pull/8926
+# The above build_ext has been run with numpy already installed in the environment
+# because otherwise it would have failed.
+# But as we run pip install without dependencies, we have to force
+# GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY=YES to disable the related check.
+# This is OK here since the bindings have already been built, and it is just
+# a matter of bundling them in the wheel.
+export GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY=YES
$PYTHON -m pip install --no-deps --ignore-installed .
--
2.25.1

2 changes: 2 additions & 0 deletions ci/travis/conda/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ cd gdal-feedstock
patch -p1 < ../ci/travis/conda/build.sh.patch
patch -p1 --binary < ../ci/travis/conda/bld.bat.patch

patch -p1 < ../ci/travis/conda/0001-Fix-build-of-Python-bindings-due-to-https-github.com.patch

cat > recipe/recipe_clobber.yaml <<EOL
source:
path: ../../../gdal
Expand Down
2 changes: 1 addition & 1 deletion ci/travis/osx/before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ conda update -n base -c defaults conda
conda install -y compilers automake pkgconfig cmake

conda config --set channel_priority strict
conda install --yes --quiet proj python=3.8 swig lxml jsonschema -y
conda install --yes --quiet proj python=3.8 swig lxml jsonschema numpy -y
conda install --yes --quiet libgdal=3.7.0=hc13fe4b_4 --only-deps -y
4 changes: 4 additions & 0 deletions ci/travis/osx/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ find ${CONDA_PREFIX}/lib -name '*.la' -delete
# build GDAL
mkdir build
cd build

# to have an environment where we test this...
export GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY=YES

# Disable Arrow/Parquet because the VM provides libraries in /usr/local/lib/
# that cause Illegal instruction error when running tests. I suspect the
# Arrow/Parquet libraries to be built with AVX2 support but the VM worker
Expand Down
27 changes: 22 additions & 5 deletions swig/python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ if (Python_Interpreter_FOUND)
endif ()

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/README.rst ${CMAKE_CURRENT_BINARY_DIR}/README.rst @ONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/pyproject.toml ${CMAKE_CURRENT_BINARY_DIR}/pyproject.toml @ONLY)

file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/get_suffix.py
"from importlib.machinery import EXTENSION_SUFFIXES; print(EXTENSION_SUFFIXES[0])\n")
Expand All @@ -203,8 +204,11 @@ if (Python_Interpreter_FOUND)
GENERATE
OUTPUT "${BUILD_EXT_FILENAME}"
CONTENT
"file(REMOVE_RECURSE ${CMAKE_CURRENT_BINARY_DIR}/build)\n
execute_process(COMMAND $<IF:$<NOT:$<CONFIG:Debug>>,\"${Python_EXECUTABLE_CMAKE}\" \"${SETUP_PY_FILENAME}\" build_ext --inplace,${CMAKE_COMMAND} -E echo \"setup.py build_ext only run in configuration != Debug\">)\n"
"file(REMOVE_RECURSE ${CMAKE_CURRENT_BINARY_DIR}/build)
execute_process(COMMAND $<IF:$<NOT:$<CONFIG:Debug>>,\"${Python_EXECUTABLE_CMAKE}\" \"${SETUP_PY_FILENAME}\" build_ext --inplace,${CMAKE_COMMAND} -E echo \"setup.py build_ext only run in configuration != Debug\"> RESULT_VARIABLE res)
if(NOT res EQUAL 0)
message(FATAL_ERROR \"setup.py bdist_wheel failed\")
endif()"
)
else ()
set(BUILD_EXT_FILENAME "${CMAKE_CURRENT_BINARY_DIR}/build_ext.cmake")
Expand All @@ -213,7 +217,11 @@ if (Python_Interpreter_FOUND)
OUTPUT "${BUILD_EXT_FILENAME}"
CONTENT
"file(REMOVE_RECURSE ${CMAKE_CURRENT_BINARY_DIR}/build)\n
execute_process(COMMAND \"${Python_EXECUTABLE_CMAKE}\" \"${SETUP_PY_FILENAME}\" build_ext --inplace)\n")
execute_process(COMMAND \"${Python_EXECUTABLE_CMAKE}\" \"${SETUP_PY_FILENAME}\" build_ext --inplace RESULT_VARIABLE res)
if(NOT res EQUAL 0)
message(FATAL_ERROR \"setup.py bdist_wheel failed\")
endif()"
)
endif ()

if(CMAKE_NM AND BUILD_SHARED_LIBS AND NOT _isMultiConfig)
Expand Down Expand Up @@ -300,6 +308,7 @@ if (Python_Interpreter_FOUND)
symlink_or_copy("${CMAKE_CURRENT_SOURCE_DIR}/gdal-utils" "${INSTALL_WORKING_DIRECTORY}/gdal-utils")
set(BUILD_EXT_WITH_RPATH_CONTENT)
string(APPEND BUILD_EXT_WITH_RPATH_CONTENT "configure_file(\"${CMAKE_CURRENT_SOURCE_DIR}/README.rst\" \"${INSTALL_WORKING_DIRECTORY}/README.rst\" @ONLY)\n")
string(APPEND BUILD_EXT_WITH_RPATH_CONTENT "configure_file(\"${CMAKE_CURRENT_SOURCE_DIR}/pyproject.toml\" \"${INSTALL_WORKING_DIRECTORY}/pyproject.toml\" @ONLY)\n")
string(APPEND BUILD_EXT_WITH_RPATH_CONTENT "file(COPY \"${CMAKE_CURRENT_BINARY_DIR}/extensions\" DESTINATION \"${INSTALL_WORKING_DIRECTORY}\")\n")
string(APPEND BUILD_EXT_WITH_RPATH_CONTENT "file(GLOB PY_FILES \"${CMAKE_CURRENT_BINARY_DIR}/osgeo/*.py\")\n")
string(APPEND BUILD_EXT_WITH_RPATH_CONTENT "foreach(_file IN LISTS PY_FILES)\n")
Expand Down Expand Up @@ -336,11 +345,19 @@ if (Python_Interpreter_FOUND)
GENERATE
OUTPUT ${BUILD_BDIST_WHEEL_FILENAME}
CONTENT
"execute_process(COMMAND $<IF:$<NOT:$<CONFIG:Debug>>,${Python_EXECUTABLE_CMAKE} ${SETUP_PY_FILENAME} bdist_wheel,${CMAKE_COMMAND} -E echo \"setup.py bdist_wheel only run in configuration != Debug\">)"
"execute_process(COMMAND $<IF:$<NOT:$<CONFIG:Debug>>,${Python_EXECUTABLE_CMAKE} ${SETUP_PY_FILENAME} bdist_wheel,${CMAKE_COMMAND} -E echo \"setup.py bdist_wheel only run in configuration != Debug\"> RESULT_VARIABLE res)
if(NOT res EQUAL 0)
message(FATAL_ERROR \"setup.py bdist_wheel failed\")
endif()
"
)
else ()
set(BUILD_BDIST_WHEEL_FILENAME ${CMAKE_CURRENT_BINARY_DIR}/build_bdist_wheel.cmake)
file(WRITE ${BUILD_BDIST_WHEEL_FILENAME} "execute_process(COMMAND ${Python_EXECUTABLE_CMAKE} ${SETUP_PY_FILENAME} bdist_wheel)\n")
file(WRITE ${BUILD_BDIST_WHEEL_FILENAME}
"execute_process(COMMAND ${Python_EXECUTABLE_CMAKE} ${SETUP_PY_FILENAME} bdist_wheel RESULT_VARIABLE res)
if(NOT res EQUAL 0)
message(FATAL_ERROR \"setup.py bdist_wheel failed\")
endif()")
endif ()
add_custom_target(
python_wheel
Expand Down
10 changes: 8 additions & 2 deletions swig/python/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,23 @@ GDAL can be installed from the `Python Package Index <https://pypi.org/project/G

::

pip install GDAL
pip install gdal

It will be necessary to have libgdal and its development headers installed
if pip is expected to do a source build because no wheel is available
for your specified platform and Python version.

To install with numpy support, you need to require the optional numpy component:

::

pip install gdal[numpy]

To install the version of the Python bindings matching your native GDAL library:

::

pip install GDAL=="$(gdal-config --version).*"
pip install gdal=="$(gdal-config --version).*"

Choose a reason for hiding this comment

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

Oh I think this does something I have not accounted for: Does the trailing .* account for versions like 3.8.1.1 where a new version of the Python package has been released to fix some bug with the package itself? I have always done "gdal==$(gdal-config --version)".

Copy link
Member Author

Choose a reason for hiding this comment

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

Does the trailing .* account for versions like 3.8.1.1

yes


Building as part of the GDAL library source tree
------------------------------------------------
Expand Down
42 changes: 42 additions & 0 deletions swig/python/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[build-system]
requires = ["setuptools>=61.0.0", "oldest-supported-numpy", "wheel"]
build-backend = "setuptools.build_meta"

[project]
name = "GDAL"
dynamic = ["version", "scripts"]
authors = [
{name = "Frank Warmerdam"},
{name = "Howard Butler"},
{name = "Even Rouault"},
Comment on lines +8 to +11

Choose a reason for hiding this comment

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

I see a few fields that are duplicated between setup.py and pyproject.toml. You should be able to introduce a setup.cfg file and define common bits of metadata there to avoid having to update both setup.py and pyproject.toml. I'm pretty sure the setuptools.setup() call in setup.py will use the contents of setup.cfg as defaults, and override with whatever parameters are still provided via the setup() function. I can't think of any adverse effects in terms of maintainability, but I also have never dealt with a project containing all 3 files.

Copy link
Member Author

Choose a reason for hiding this comment

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

You should be able to introduce a setup.cfg file

sigh, yet another config file... I'd prefer sticking with the current 2 file setup, unless we see that it is broken...

Copy link
Member

@avalentino avalentino Dec 7, 2023

Choose a reason for hiding this comment

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

I think that you no longer need setup.cfg if you use

[build-system]
requires = ["setuptools>=61.0.0", "wheel"]

Almost all the metadata can be simply moved from setup.py to pyproject.toml leaving in setup.py only the part defining the Extensions and the logic that cannot be expressed in a declarative manner.

]
maintainers = [
{name = "GDAL contributors", email = "gdal-dev@lists.osgeo.org"},
]
description = "GDAL: Geospatial Data Abstraction Library"
readme = "README.rst"
keywords = ["gis", "raster", "vector"]
license = {text = "MIT"}
classifiers = [
"Development Status :: 5 - Production/Stable",
"Intended Audience :: Developers",
"Intended Audience :: Science/Research",
"License :: OSI Approved :: MIT License",
"Operating System :: OS Independent",
"Programming Language :: Python :: 3",
"Programming Language :: C",
"Programming Language :: C++",
"Topic :: Scientific/Engineering :: GIS",
"Topic :: Scientific/Engineering :: Information Analysis",
]
requires-python = ">=3.8"

[project.optional-dependencies]
numpy = ['numpy>1.0.0']

[project.urls]
Homepage = "https://gdal.org"
Documentation = "https://gdal.org"
Repository = "https://github.com/OSGeo/GDAL.git"
Changelog = "https://github.com/OSGeo/gdal/blob/master/NEWS.md"
Issues = "https://github.com/OSGeo/gdal/issues"
16 changes: 11 additions & 5 deletions swig/python/setup.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,21 @@ if sys.platform == 'win32':
# ---------------------------------------------------------------------------

numpy_include_dir = '.'
numpy_error_msg = ""
try:
numpy_include_dir = get_numpy_include()
HAVE_NUMPY = numpy_include_dir != '.'
if not HAVE_NUMPY:
print("WARNING: numpy found, but numpy headers were not found! Array support will not be enabled")
numpy_error_msg = "numpy found, but numpy headers were not found!"
except ImportError:
HAVE_NUMPY = False
print('WARNING: numpy not available! Array support will not be enabled')
numpy_error_msg = "numpy not available!"

if not HAVE_NUMPY:
if "GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY" in os.environ:
print("WARNING: " + numpy_error_msg + " Array support will not be enabled.")
else:
raise Exception(numpy_error_msg + " This error may happen if you build/install using setup.py directly, but should normally not happen if you install using pip install. If you still want to build the bindings without numpy support, define the GDAL_PYTHON_BINDINGS_WITHOUT_NUMPY environment variable")

class gdal_ext(build_ext):

Expand Down Expand Up @@ -369,8 +375,8 @@ readme = open('README.rst', encoding="utf-8").read()
name = 'GDAL'
author = "Frank Warmerdam"
author_email = "warmerdam@pobox.com"
maintainer = "Howard Butler"
maintainer_email = "hobu.inc@gmail.com"
maintainer = "GDAL contributors"
maintainer_email = "gdal-dev@lists.osgeo.org"
description = "GDAL: Geospatial Data Abstraction Library"
license_type = "MIT"
url = "http://www.gdal.org"
Expand Down Expand Up @@ -426,7 +432,7 @@ setup(
packages=packages,
package_dir=package_dir,
url=url,
python_requires='>=3.6.0',
python_requires='>=3.8.0',
ext_modules=ext_modules,
scripts=glob(utils_package_root + '/scripts/*.py'),
# This must *not* be conditionalized by HAVE_NUMPY, since this is for a "pip install gdal[numpy]" type of installation
Expand Down
Loading