Skip to content

Feature/metk 176 port pymetkit to pybind11 - #239

Open
tbkr wants to merge 8 commits into
developfrom
feature/metk-176-port-pymetkit-to-pybind11
Open

Feature/metk 176 port pymetkit to pybind11#239
tbkr wants to merge 8 commits into
developfrom
feature/metk-176-port-pymetkit-to-pybind11

Conversation

@tbkr

@tbkr tbkr commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

Porting the existing pymetkit layer to pybind11

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.

📋 Metkit Documentation 📋
https://sites.ecmwf.int/docs/metkit/pull-requests/PR-239

@tbkr
tbkr marked this pull request as draft July 23, 2026 14:59
@tbkr
tbkr force-pushed the feature/metk-176-port-pymetkit-to-pybind11 branch from 1e2b6de to 83195a3 Compare July 23, 2026 15:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR ports the pymetkit Python layer from the legacy CFFI approach to a new pybind11-based extension module, adds a small Pythonic wrapper API, and wires packaging/tests/docs into the CMake and CI workflows.

Changes:

  • Add pymetkit_bindings (pybind11) + a Pythonic pymetkit layer (MarsRequest, MarsSelection, parse_mars_request) built and packaged from the CMake build.
  • Add a new tests/pymetkit pytest suite (integration tests for request construction/access/expand/merge/parse + CLI tests) and hook it into ctest.
  • Add/refresh docs/pymetkit for installation, API, bindings, examples, and legacy migration notes; mark the old CFFI code as deprecated.

mars2grib documentation sync (CodingGuidelineID: 1000000):

  • Impacted files under src/metkit/mars2grib: none
  • Impacted files under tests/mars2grib: none
  • Documentation status for these paths: ✅ in sync (no affected files)

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/pymetkit/integration/mars_request/test_selection.py Adds integration tests for MarsSelection normalization and validation.
tests/pymetkit/integration/mars_request/test_parse.py Adds integration tests for parsing requests from strings/files.
tests/pymetkit/integration/mars_request/test_merge.py Adds integration tests for request merging semantics and error cases.
tests/pymetkit/integration/mars_request/test_expand.py Adds integration tests for expansion/default inheritance and validation.
tests/pymetkit/integration/mars_request/test_equality.py Adds integration tests for equality and repr behavior.
tests/pymetkit/integration/mars_request/test_construction.py Adds integration tests for construction sources and value normalization.
tests/pymetkit/integration/mars_request/test_accessors.py Adds integration tests for mapping-like access/mutation APIs.
tests/pymetkit/CMakeLists.txt Registers pytest tests with CTest and configures test environment.
tests/pymetkit/cli/test_cli.py Adds tests for the python -m pymetkit diagnostic CLI behavior/logging.
tests/CMakeLists.txt Conditionally includes tests/pymetkit when the feature is enabled.
src/pymetkit/README.md Documents architecture/usage for the new pybind11-based package.
src/pymetkit/pymetkit.py Implements Pythonic MarsRequest wrapper and parse_mars_request.
src/pymetkit/pymetkit_type.py Implements MarsSelection with normalization helpers.
src/pymetkit/_internal/pymetkit_internal.py Initializes bindings and defines MetKitException.
src/pymetkit/_internal/init.py Loads libmetkit via findlibs and re-exports binding symbols.
src/pymetkit/main.py Adds CLI for dependency/home/version diagnostics.
src/pymetkit/init.py Defines the public import surface (MarsRequest, etc.).
src/pymetkit_bindings/CMakeLists.txt Builds the pybind11 extension and stages the output.
src/pymetkit_bindings/bindings.cc Implements pybind11 bindings for metkit::mars::MarsRequest + parsing/version info.
src/CMakeLists.txt Adds staging layout + wheel build target for pymetkit.
python/pymetkit/tests/test_marsrequest.py Minor formatting tweaks in legacy tests file.
python/pymetkit/src/pymetkit/pymetkit.py Marks legacy CFFI module as deprecated (docstring).
python/pymetkit/README.md Marks legacy CFFI interface as deprecated and points to new package/docs.
pyproject.toml Removes the legacy pyproject-based packaging configuration.
docs/pymetkit/requirements.txt Adds Sphinx/doc build requirements for PyMetKit docs.
docs/pymetkit/legacy.rst Documents the legacy CFFI interface and migration notes.
docs/pymetkit/installation.rst Documents build/install flows and findlibs diagnostics.
docs/pymetkit/index.rst Updates landing page and adds quick start/toctree structure.
docs/pymetkit/examples.rst Adds doctestable usage examples for the new API.
docs/pymetkit/development.rst Adds development workflow notes (wheel building, env vars, docs).
docs/pymetkit/conftest.py Configures Sybil to run doc examples as tests.
docs/pymetkit/conf.py Adds Sphinx configuration (autoapi, theme, exclusions).
docs/pymetkit/build_docs.sh Adds a script to build the Sphinx documentation.
docs/pymetkit/bindings.rst Documents the low-level pybind11 binding surface.
docs/pymetkit/api.rst Documents the Pythonic API via sphinx-autoapi.
CMakeLists.txt Adds a new PYTHON_METKIT_INTERFACE feature option.
cmake/pymetkit_setup.py.in Adds setup.py template for wheel builds from the staged layout.
cmake/pymetkit_setup.cfg.in Adds setup.cfg template (metadata/extras/python_requires).
.github/workflows/pymetkit.yml Adds a CI workflow to build/test wheels across Python versions.
.github/workflows/cd.yml Removes the wheel-python CD job invocation.
Comments suppressed due to low confidence (1)

tests/pymetkit/integration/mars_request/test_parse.py:104

  • The test opens a file handle without closing it (parse_mars_request(open(...))). Use a context manager so the handle is reliably closed on all platforms.

Comment thread src/pymetkit/_internal/__init__.py Outdated
Comment thread cmake/pymetkit_setup.py.in
Comment thread src/pymetkit_bindings/CMakeLists.txt Outdated
Comment thread src/pymetkit_bindings/bindings.cc
Comment thread cmake/pymetkit_setup.py.in
Comment thread src/pymetkit/pymetkit.py
Comment thread tests/pymetkit/integration/mars_request/test_parse.py Outdated
@tbkr
tbkr force-pushed the feature/metk-176-port-pymetkit-to-pybind11 branch from 83195a3 to 9b15ffb Compare July 30, 2026 09:34
@codecov-commenter

codecov-commenter commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.96%. Comparing base (2d8a915) to head (b87fe02).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #239   +/-   ##
========================================
  Coverage    50.96%   50.96%           
========================================
  Files          384      384           
  Lines        15461    15461           
  Branches      1309     1309           
========================================
  Hits          7879     7879           
  Misses        7582     7582           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tbkr
tbkr force-pushed the feature/metk-176-port-pymetkit-to-pybind11 branch from 9b15ffb to 8c0686f Compare July 30, 2026 11:13
@tbkr
tbkr marked this pull request as ready for review July 30, 2026 11:13
Comment thread src/pymetkit/pymetkit.py
Comment thread src/pymetkit/pymetkit_type.py Outdated
Comment thread tests/pymetkit/CMakeLists.txt
Comment thread src/pymetkit/pymetkit.py
Comment thread src/pymetkit_bindings/bindings.cc Outdated
Comment thread src/pymetkit/pymetkit.py
Comment thread src/pymetkit/pymetkit.py Outdated
Comment thread src/pymetkit/__main__.py
Comment thread cmake/pymetkit_setup.py.in
Comment thread python/pymetkit/src/pymetkit/pymetkit.py
Comment thread src/pymetkit/pymetkit_type.py
Also remove the inheritance from dict for the MarsRequest
@tbkr
tbkr force-pushed the feature/metk-176-port-pymetkit-to-pybind11 branch from 91a0af6 to 2833644 Compare August 3, 2026 14:16
@tbkr
tbkr force-pushed the feature/metk-176-port-pymetkit-to-pybind11 branch from 26c8f3c to bdaf8f2 Compare August 3, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants