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

Update CMake and setuptools logic for the Python bindings #233

Merged
merged 18 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 41 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,18 @@ jobs:
working-directory: ${{runner.workspace}}/build
run: make test

pybind11:
pybind11-pip:
needs: [build-ubuntu, build-mac]
strategy:
fail-fast: false
matrix:
platform: [macos-latest, ubuntu-latest] #windows-latest,
# python-version: [3.5, 3.6, 3.7, 3.8]
python-version: [3.6]
python-version: [3.6, 3.7, 3.8, 3.9]
runs-on: ${{ matrix.platform }}
steps:
- name: Checkout
uses: actions/checkout@v2
- run: git fetch --prune --unshallow
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
Expand All @@ -235,12 +235,46 @@ jobs:
- name: Setup
run: |
python -m pip install --upgrade pip
pip install pytest "pybind11[global]"
pip install -r requirements.txt
python -m pip install build
- name: Build
run: pip install .
run: python -m pip install -v .[testing]
- name: Test
run: pytest
run: python -m pytest

pybind11-cmake:
needs: [build-ubuntu, build-mac]
# strategy:
# fail-fast: false
# matrix:
# python-version: [3.9]
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- run: git fetch --prune --unshallow
# - name: Set up Python ${{ matrix.python-version }}
# uses: actions/setup-python@v2
# with:
# python-version: ${{ matrix.python-version }}
- name: Setup apt
run: |
sudo apt update
sudo apt install -y libeigen3-dev pybind11-dev python3-pytest python3-numpy
mkdir ${{runner.workspace}}/build
- name: Configure
working-directory: ${{runner.workspace}}/build
run: cmake $GITHUB_WORKSPACE -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=ON -DBUILD_PYTHON_BINDINGS=ON -DCMAKE_BUILD_TYPE=Release
- name: Build
working-directory: ${{runner.workspace}}/build
run: make -j2
- name: Test
working-directory: ${{runner.workspace}}/build
run: make test
- name: Install
working-directory: ${{runner.workspace}}/build
run: sudo make install
- name: Test Import
run: python3 -c 'import manifpy'

# arm64:
# needs: [build-ubuntu]
Expand Down
72 changes: 72 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
name: release
on:
push:
tags:
- '*'
pull_request:
branches:
- devel # master only when ready
- master
workflow_dispatch:

jobs:

build-sdist:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- run: git fetch --prune --unshallow

- name: Install Python
uses: actions/setup-python@v2
with:
python-version: 3.6

- name: Setup apt
run: |
sudo apt update
sudo apt install -y libeigen3-dev

- name: Setup
run: |
python3 -m pip install --upgrade pip
pip3 install build

- name: Build sdist
run: python3 -m build --sdist -o dist/

- name: Build wheel
run: python3 -m build --wheel -o dist/

- name: Upload artifacts
uses: actions/upload-artifact@v2
with:
name: dist
path: dist/*
# path: |
# path/*.whl
# path/*.tar.gz

upload_pypi:
needs: build-sdist
runs-on: ubuntu-latest
steps:

- uses: actions/download-artifact@v2
with:
name: dist
path: dist

- name: Inspect dist folder
run: ls -lah dist/

# @todo: see https://github.com/diegoferigo/manif/pull/1#discussion_r668531581
# - uses: pypa/gh-action-pypi-publish@master
# if: |
# github.repository == 'artivis/manif' &&
# ((github.event_name == 'release' && github.event.action == 'published') ||
# (github.event_name == 'push' && github.ref == 'refs/heads/main'))
# with:
# user: __token__
# password: ${{ secrets.PYPI_TOKEN }}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ docs/m.css
*.so
*__pycache__
.pytest_cache
dist
Comment on lines 15 to +18
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the long term, you can consider using the official gitignore for Python:

https://github.com/github/gitignore/blob/master/Python.gitignore

3 changes: 1 addition & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ Let's install all dependencies for development and testing,

```terminal
apt install libeigen3-dev
python3 -m pip install "pybind11[global]" pytest
python3 -m pip install -r requirements
python3 -m pip install "pybind11[global]" pytest numpy
```

We can now build **manif**, its Python wrappers and all tests,
Expand Down
50 changes: 37 additions & 13 deletions docs/pages/python/Quick-start.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
# Quick start

- [Quick start](#quick-start)
- [Getting Pybind11](#getting-pybind11)
- [Installation](#installation)
- [Dependencies](#dependencies)
- [Installing manifpy](#installing-manifpy)
- [From conda](#from-conda)
- [From source](#from-source)
- [Getting Pybind11](#getting-pybind11)
- [Getting the dependencies](#getting-the-dependencies)
- [Building](#building)
- [Testing](#testing)
- [Use manifpy in your project](#use-manifpy-in-your-project)
- [Tutorials and application demos](#tutorials-and-application-demos)

## Getting Pybind11
## Installing manifpy

### From conda

`manifpy` can be installed from the [conda-forge][conda-manifpy],

```bash
conda install -c conda-forge manifpy
```

### From source

#### Getting Pybind11

The Python wrappers are generated using [pybind11][pybind11-rtd]. So first we need to install it,
but we want it available directly in our environment root so that `CMake` can find it.
Expand All @@ -30,9 +45,9 @@ cmake ..
make install
```

## Installation
<!-- ## Installation -->

### Dependencies
#### Getting the dependencies

- Eigen 3 :
- Linux ( Ubuntu and similar )
Expand All @@ -49,13 +64,7 @@ make install

- [lt::optional][optional-repo] : included in the `external` folder

Python bindings also depends on `numpy`.

```bash
python3 -m pip install -r requirements
```

### From source
#### Building

To generate `manif` Python bindings run,

Expand All @@ -65,6 +74,20 @@ cd manif
python3 -m pip install .
```

#### Testing

To run the tests you will also need `numpy`,

```bash
python3 -m pip install numpy
```

To run the tests, simply hits:

```bash
python3 -m pytest
```

## Use manifpy in your project

```python
Expand Down Expand Up @@ -101,3 +124,4 @@ python3 se2_localization.py

[pybind11-rtd]: https://pybind11.readthedocs.io/en/stable/index.html
[optional-repo]: https://github.com/TartanLlama/optional
[conda-manifpy]: https://anaconda.org/conda-forge/manifpy
29 changes: 0 additions & 29 deletions docs/pages/python/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,6 @@ Quick start
.. contents:: Table of Contents
:depth: 3


Getting Pybind11
----------------

The Python wrappers are generated using `pybind11 <https://pybind11.readthedocs.io/en/stable/index.html>`_. So first we need to install it,
but we want it available directly in our environment root so that ``CMake`` can find it.
To do so we can use,

.. code-block:: bash

python3 -m pip install "pybind11[global]"

Note that this is not recommended when using one's system Python,
as it will add files to ``/usr/local/include/pybind11`` and ``/usr/local/share/cmake/pybind11``.

Another way is to use ``CMake`` to install it,

.. code-block:: bash

git clone https://github.com/pybind/pybind11.git
cd pybind11 && mkdir build && cd build
cmake ..
make install

Installation
------------

Expand Down Expand Up @@ -60,11 +36,6 @@ Dependencies
*
`lt::optional <https://github.com/TartanLlama/optional>`_ : included in the ``external`` folder

Python bindings also depends on ``numpy``.

.. code-block:: bash

python3 -m pip install -r requirements

From source
^^^^^^^^^^^
Expand Down
14 changes: 14 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[build-system]
requires = [
"wheel",
"setuptools>=45",
"setuptools_scm[toml]>=6.0",
"ninja",
"cmake>=3.18.2",
artivis marked this conversation as resolved.
Show resolved Hide resolved
"cmake-build-extension",
"pybind11",
]
build-backend = "setuptools.build_meta"

[tool.setuptools_scm]
local_scheme = "dirty-tag"
2 changes: 0 additions & 2 deletions pytest.ini

This file was deleted.

50 changes: 49 additions & 1 deletion python/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
set(PYBIND11_CPP_STANDARD -std=c++11)
pybind11_add_module(manifpy
pybind11_add_module(manifpy MODULE
Copy link
Owner

Choose a reason for hiding this comment

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

Note to myself, MODULE is the default when nothing's specified.

bindings_rn.cpp
bindings_so2.cpp
bindings_so3.cpp
Expand All @@ -17,3 +17,51 @@ target_compile_definitions(manifpy PRIVATE EIGEN_DEFAULT_TO_ROW_MAJOR)
set_property(TARGET manifpy PROPERTY CXX_STANDARD 11)
set_property(TARGET manifpy PROPERTY CXX_STANDARD_REQUIRED ON)
set_property(TARGET manifpy PROPERTY CXX_EXTENSIONS OFF)

if (CALL_FROM_SETUP_PY)
# cmake-build-extension sets the full absolute path as CMAKE_INSTALL_PREFIX.
set(MANIFPY_INSTDIR "${CMAKE_INSTALL_PREFIX}")
else()
# 'distutils.sysconfig.get_python_lib' returns the absolute path of Python
# by default a global location managed by the distro e.g. /usr/lib/python.
#
# pybind11 and FindPython3 set respectively PYTHON_SITE_PACKAGES/Python3_SITELIB
# from 'distutils.sysconfig.get_python_lib'
#
# Those are especially annoying on Ubuntu since it has
# some hardcoded paths in python3.x/site.py
#
# `sysconfig.get_path` may return paths that does not even exists.
#
# So below we retrieve the first site-package path from 'site.getsitepackages()'.

execute_process(
COMMAND
${PYTHON_EXECUTABLE} -c "import site; print(site.getsitepackages()[0])"
OUTPUT_VARIABLE _PYTHON_SITE_PACKAGE OUTPUT_STRIP_TRAILING_WHITESPACE
)
set(MANIFPY_INSTDIR "${_PYTHON_SITE_PACKAGE}/manifpy")
endif()

message(STATUS "Installing manifpy in ${MANIFPY_INSTDIR}")

# Setup installation path
install(TARGETS manifpy COMPONENT python DESTINATION "${MANIFPY_INSTDIR}")

# Create the Python package in the build tree for testing purposes
set(MANIFPY_BUILDDIR "${CMAKE_BINARY_DIR}/manifpy")
set_target_properties(
manifpy PROPERTIES
OUTPUT_NAME _bindings
LIBRARY_OUTPUT_DIRECTORY "${MANIFPY_BUILDDIR}")

# Create the __init__.py file
file(
GENERATE
OUTPUT "${MANIFPY_BUILDDIR}/__init__.py"
CONTENT "from manifpy._bindings import *\n")

# Install the __init__.py file
install(
FILES "${MANIFPY_BUILDDIR}/__init__.py"
DESTINATION ${MANIFPY_INSTDIR})
2 changes: 1 addition & 1 deletion python/bindings_manif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ void wrap_SE3(pybind11::module &m);

void wrap_SE_2_3(pybind11::module &m);

PYBIND11_MODULE(manifpy, m) {
PYBIND11_MODULE(_bindings, m) {
artivis marked this conversation as resolved.
Show resolved Hide resolved
m.doc() = "Python bindings for the manif library, "
"a small library for Lie theory.";

Expand Down
1 change: 0 additions & 1 deletion requirements.txt

This file was deleted.

Loading