Gh actions for python #1721

BDoignies · 2024-04-17T08:16:36Z

PR Description

This PR proposes a redesign of the CI pipeline for Python binding. Mainly, this PR changes the CI backend from Azure to Github Actions.

I am not so sure about the review of this PR though, because it was tested on my repository and that some of the changes are repository-dependent (see below for a full list).

Quick overview of changes

Remove files of Azure pipeline (they are no longer required, but in the same time this invalidates previous CI for this PR...).
Binding files (.h) have an extra definition to make sure it compiles on Windows. The ssize_t variable is only POSIX and not standard. Hence Windows does not have it (technically #include <BaseTsd.h> is possible, but I find it to be too heavy for a simple define).
wrap/__init__.py -> wrap/deploy/__init__.py. Previous location caused issues with CTest. As stated in pytest good practice, import are done differently when running tests.
New .yml files (see below)

Some 'pending changes'

Package name in setup.py : for now it is configured as 'dgtal_githubactions_test' for testing purposes
DGtal build options: this PR uses (for now) defaults options most of the time (except for windows where conan is necessary). A maintainer review on this would be more than helpful
Python versions and build matrix: What python version should be supported, on what oses, ...
Some tests commented: The goal of this PR is only about CI, not the tests themselves. Some tests did not pass not because of the setup (afaik) but because of flawed design. I commented them to make sure the workflow would work properly.

Repository-dependent changes

File pythonBindings-PR.yml is repository-independent and should run on any other fork. However the Pypi deployment is much more different.

Some documentation about the process :

Currently the file is setup to:

Run on tag pushes: It extract the tagname and put it automatically as the version for the package. This might be a risk if tags are used for other reasons in the future.
Run on an environment called 'deploy'. This is repository dependent and other names might be used in the future. Personally, I encourage maintainers to use an environment that requires 'manual' validation for build and upload to pypi.
Push the wheel to TestPypi which is on the test pypi and owned by me (sorry for 'stealing the name', is here for test purposes only and will be removed when this PR is merged of if asked by owners of the library). Before changing the links, make sure Trusted publishers is setup for this repository.

What I did not test :

MacOS and Windows pip install : https://test.pypi.org/manage/project/dgtal-ghactions-test/releases/. I'd appreciate if anyone could test that as I did not specifically asked for static linking of libraries (linux manywheel exports libraries inside the wheel).

What's missing:

Documentation: I did not rewrite the documentation yet. I am waiting for any comments reviewers might have.
Github releases: It is possible to publish directly the .wheel and make them available as part of the tags.
Comments and reviews: I'm am neither an expert of Dgtal nor an expert of CI and python build. Any comments is welcome to make sure everything is fine and for the best 😺 !

Checklist

Most of the following are not applicable to this PR.

[?] Unit-test of your feature with Catch.
[?] Doxygen documentation of the code completed (classes, methods, types, members...)
[?] Documentation module page added or updated.
New entry in the ChangeLog.md added.
[?] No warning raised in Debug mode.
[?] All continuous integration tests pass (Github Actions)

dcoeurjo · 2024-05-08T09:30:26Z

Many thanks for the PR. Just a first test, I got a Python.h error on my Mac:

/Users/davidcoeurjolly/Sources/DGtal/build/_deps/pybind11-src/include/pybind11/detail/common.h:213:10: fatal error: 'Python.h' file not found
#include <Python.h>
         ^~~~~~~~~~

It reminds me something similar for UTK, I'd need to investigate a bit.

dcoeurjo · 2024-05-08T09:42:27Z

Ok something I should debug:


-- Found PythonInterp: /Users/davidcoeurjolly/anaconda3/python.app/Contents/MacOS/python (found version "3.11.4")
-- Found PythonLibs: /Users/davidcoeurjolly/anaconda3/lib/libpython3.11.dylib
-- Performing Test HAS_FLTO
-- Performing Test HAS_FLTO - Success
-- Performing Test HAS_FLTO_THIN
-- Performing Test HAS_FLTO_THIN - Success
-- Tests for Python-only ENABLED
-- Configuring done (4.2s)
CMake Error in wrap/CMakeLists.txt:
  Imported target "pybind11::module" includes non-existent path

    "/Users/davidcoeurjolly/anaconda3/python.app/Contents/include/python3.11"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.



CMake Error in wrap/CMakeLists.txt:
  Imported target "pybind11::module" includes non-existent path

    "/Users/davidcoeurjolly/anaconda3/python.app/Contents/include/python3.11"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

dcoeurjo · 2024-05-08T17:05:59Z

(ok problem solved.. just a conda issue)

dcoeurjo · 2024-05-08T18:10:07Z

wrap/tests/images/test_ImageContainerBySTLVector.py

@@ -1,6 +1,7 @@
 import pytest
 import dgtal

+"""


Why such string statement ?

This the pythonic way of doing multiline comments. As mentionned in the PR, those test do not run -- and this is not because of GH actions.

dcoeurjo · 2024-05-08T18:10:50Z

Very nice PR, thx @BDoignies . We should discuss a bit about this pypi thing..

dcoeurjo · 2024-05-08T18:14:16Z

In the build directory, how can I just test the python binding ? Just import dgtal in this folder?

BDoignies · 2024-05-22T09:22:57Z

You can follow the ".github/workflows/pythonBindings-PR.yml" from line 92 and onwards if you have any more questions about the test process. Here are the lines answering your question :

In the build directory, run

ctest -R python -C ${{ matrix.BUILD_TYPE }} -V --output-on-failure

You can also test that the module can be properly imported with the line :

python -c "import dgtal"

BDoignies · 2024-05-22T09:24:29Z

We should discuss a bit about this pypi thing..

You know where to find / contact me to setup a meeting for this !

dcoeurjo · 2024-05-22T10:47:21Z

You know where to find / contact me to setup a meeting for this !

I will ;)

dcoeurjo

Merging to test the deploy and GH

BDoignies · 2024-05-24T09:31:11Z

Just to make sure information is somewhere in case it is needed.

Reasons for Pypi deployment to break:

Tags are not appropriate version numbers. Fix: either change tag or update the workflow file to filter and extract tag information properly
Same version / already pushed files. Fix: none. You simply can not push twice the same version.
manylinux version. Because DGtal may use advanced features, the manylinux wheel should use relatively new versions. If only linux deployment breaks, check the autiwhell call for the version.

BDoignies added 8 commits April 15, 2024 17:25

Added Github Actions for python tests

973f2b1

Enabled MacOS / Windows in Python CI

a995bd9

Fixs compilation issues on windows

b246dc1

Fixs compilation issues on windows

86ab53d

Github action to deploy to pypi

8ac4290

Set environment

b6505fe

Testpypi deployment

f248f3e

Remove azure pipelines

66e1fd5

Merge branch 'master' into GHActionsPython

26ea176

dcoeurjo reviewed May 8, 2024

View reviewed changes

Merge branch 'master' into GHActionsPython

e0461e3

dcoeurjo and others added 6 commits May 23, 2024 15:18

Merge branch 'master' into GHActionsPython

07ba4a9

Merge branch 'master' into GHActionsPython

80217b4

Cleaning up pypi-gh script

bfd65ef

Pypi URLs

35a27e9

setup.py

a21e9ce

setup.py

9e873c0

dcoeurjo self-requested a review May 23, 2024 14:35

dcoeurjo approved these changes May 23, 2024

View reviewed changes

dcoeurjo merged commit 72e0379 into DGtal-team:master May 23, 2024
0 of 14 checks passed

BDoignies deleted the GHActionsPython branch May 24, 2024 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gh actions for python #1721

Gh actions for python #1721

BDoignies commented Apr 17, 2024

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

dcoeurjo May 8, 2024

BDoignies May 22, 2024

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

BDoignies commented May 22, 2024

BDoignies commented May 22, 2024 •

edited

Loading

dcoeurjo commented May 22, 2024

dcoeurjo left a comment

BDoignies commented May 24, 2024

Gh actions for python #1721

Gh actions for python #1721

Conversation

BDoignies commented Apr 17, 2024

PR Description

Quick overview of changes

Some 'pending changes'

Repository-dependent changes

What I did not test :

What's missing:

Checklist

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

dcoeurjo May 8, 2024

Choose a reason for hiding this comment

BDoignies May 22, 2024

Choose a reason for hiding this comment

dcoeurjo commented May 8, 2024

dcoeurjo commented May 8, 2024

BDoignies commented May 22, 2024

BDoignies commented May 22, 2024 • edited Loading

dcoeurjo commented May 22, 2024

dcoeurjo left a comment

Choose a reason for hiding this comment

BDoignies commented May 24, 2024

BDoignies commented May 22, 2024 •

edited

Loading