Feature/adr 003 pybind11 by tbkr · Pull Request #24 · ecmwf/codex

tbkr · 2025-08-01T08:01:10Z

Description

Contributor Declaration

By opening this pull request, I affirm the following:

All authors agree to the Contributor License Agreement.
The code follows the project's coding standards.
I have performed self-review and added comments where needed.
I have added or updated tests to verify that my changes are effective and functional.
I have run all existing tests and confirmed they pass.

tmi · 2025-08-01T14:47:16Z

+while explicitly having reduced compatibility with `std` and third party modules.
+It is not header-only and requires linking to a shared or static library.
+
+[ ] Listed all seriously considered alternatives 


would we consider Cython an Option here? Some of our cpp2python interfaces are built with it (eckit, mir)

marcosbento · 2025-08-20T07:58:30Z

+1, for adoption of pybind11.

As a matter of fact, I've spent a couple of afternoons and converted ecflow's python extension from Boost.python to pybind11 (the result is still in a local branch, since the CI runners don't have pybind11).

The result is quite admirable, and the support for std type allowed to simplify parts of the existing implementation.
Moving away from Boost.python, and specifically not having to link against static libraries, is a great added bonus!

simondsmart · 2025-09-18T09:26:31Z

+ * call C functions: python code can now call C functions and manipulate C data
+   structures as if they were native Python objects
+
+Using CFFI usally requires to write a certain amount of wrapping code to hide


I would go beyond this. It typically requires a 2-layer python interface.

One layer that uses the C-interface, and one layer on top to make it more pythonic.

Reformulated the CFFI parts.

simondsmart · 2025-09-18T09:33:04Z

+## Decision
+
+The decision is to use Pybind11 for C++ bindings (see [analysis](#analysis)). 
+Developers should follow the [design guidline](#design-guidlines).


Where we do not need/envisage fortran bindings.

oiffrig

While I understand and share the enthusiasm around PyBind11, I am not sure I am convinced it is a better alternative than Cython.

simondsmart · 2025-09-22T10:36:30Z

+to call C functions and use C data types. With CFFI it is possible to
+
+* Extract C declarations from your to-be-wrapped API, note that this is a
+  preprocessing step because CFFI cannot parse C or C++ header files.


Not true. CFFI does pars C header files. But is quite restricted on what it can parse.

simondsmart · 2025-09-22T10:38:00Z

+  [here](https://pybind11.readthedocs.io/en/stable/advanced/cast/overview.html#conversion-table).
+* Allows exchanging wrapped types across bindings, see [module local
+  bindings](https://pybind11.readthedocs.io/en/stable/advanced/classes.html#module-local-class-bindings)
+* Allows setting call policies that affect object lifetimes. These policies allow to


"These policies enable the definition of dependencies ..." or similar. "allow to" is bad English.

simondsmart

Looking good. I'm happy.

tlmquintino

minor comments. needs a References section.

tlmquintino · 2025-09-22T11:52:36Z

+## Status
+
+[**Proposed** | <s>Accepted</s> | <s>Deprecated</s> | <s>Superseded by [ADR-XXX]</s>]
+


Remember to change to Accepted before merging.

tlmquintino · 2025-09-22T11:58:15Z

+[bindings](https://github.com/ecmwf/multio/blob/feature/multiom-cpp-python/src/pymars2grib_bindings/pymars2grib_bindings.cc),
+[python
+wrapper)(https://github.com/ecmwf/multio/tree/feature/multiom-cpp-python/src/pymars2grib)
+


there is a References section missing (see template).
I would suggest to add references to other similar discussions that you may or not have used, e.g:

http://blog.behnel.de/posts/cython-pybind11-cffi-which-tool-to-choose.html

https://medium.com/@iftimiealexandru/bridging-the-gap-between-python-and-c-c-libraries-an-exploration-of-integrating-with-cython-cc3bbfcfe539

oiffrig · 2025-09-22T14:01:19Z

+* Automatically translates C++ exceptions to matching Python exceptions, see
+  [details](https://pybind11.readthedocs.io/en/stable/advanced/exceptions.html).
+  This includes the ability to register custom type conversions that map domain specific
+  exceptions, e.g. `eckit::Exception` to `eckit.Excpeption`.


Suggested change

exceptions, e.g. `eckit::Exception` to `eckit.Excpeption`.

exceptions, e.g. `eckit::Exception` to `eckit.Exception`.

Co-authored-by: Kai Kratz <kai.kratz@ecmwf.int> Co-authored-by: Tobias Kremer <tobias.kremer@ecmwf.int>

tmi reviewed Aug 1, 2025

View reviewed changes

Comment thread ADR/ADR-002-PyBind11-For-CPP-Bindings.md Outdated

pgeier force-pushed the feature/ADR-002-pybind11 branch 2 times, most recently from 7c419b4 to 6141180 Compare September 18, 2025 08:29

pgeier changed the title ~~Feature/adr 002 pybind11~~ Feature/adr 003 pybind11 Sep 18, 2025

pgeier force-pushed the feature/ADR-002-pybind11 branch from 6141180 to fad5245 Compare September 18, 2025 08:32

simondsmart reviewed Sep 18, 2025

View reviewed changes

oiffrig reviewed Sep 18, 2025

View reviewed changes

pgeier force-pushed the feature/ADR-002-pybind11 branch 2 times, most recently from 15a19f8 to 79a8854 Compare September 18, 2025 14:48

oiffrig reviewed Sep 19, 2025

View reviewed changes

simondsmart reviewed Sep 22, 2025

View reviewed changes

simondsmart approved these changes Sep 22, 2025

View reviewed changes

Ozaq force-pushed the feature/ADR-002-pybind11 branch from c00f23f to 09c914a Compare September 22, 2025 10:56

tlmquintino requested changes Sep 22, 2025

View reviewed changes

Ozaq force-pushed the feature/ADR-002-pybind11 branch 2 times, most recently from 45edae9 to 61659a8 Compare September 22, 2025 13:03

oiffrig approved these changes Sep 22, 2025

View reviewed changes

New ADR - Using Pybind11 for C++ bindings

974f1ee

Co-authored-by: Kai Kratz <kai.kratz@ecmwf.int> Co-authored-by: Tobias Kremer <tobias.kremer@ecmwf.int>

Ozaq force-pushed the feature/ADR-002-pybind11 branch from 61659a8 to 974f1ee Compare September 22, 2025 14:43

simondsmart merged commit 7c3be8a into main Sep 22, 2025
1 check passed

simondsmart deleted the feature/ADR-002-pybind11 branch September 22, 2025 15:08

		## Status

		[Proposed \| <s>Accepted</s> \| <s>Deprecated</s> \| <s>Superseded by [ADR-XXX]</s>]

	exceptions, e.g. `eckit::Exception` to `eckit.Excpeption`.
	exceptions, e.g. `eckit::Exception` to `eckit.Exception`.

Conversation

tbkr commented Aug 1, 2025

Description

Contributor Declaration

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcosbento commented Aug 20, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oiffrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simondsmart left a comment

Choose a reason for hiding this comment

Uh oh!

tlmquintino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants