Rust bindings (mir-sys crate) by Choochmeque · Pull Request #49 · ecmwf/mir

Choochmeque · 2026-05-06T23:08:26Z

Description

Full implementation of the -sys crate for mir

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.

codecov-commenter · 2026-05-06T23:25:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.39%. Comparing base (0ef8096) to head (670ce1a).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop      #49   +/-   ##
========================================
  Coverage    56.39%   56.39%           
========================================
  Files          620      620           
  Lines        25891    25891           
  Branches      2279     2279           
========================================
  Hits         14601    14601           
  Misses       11290    11290

☔ View full report in Codecov by Sentry.
📢 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.

pmaciel · 2026-06-21T07:20:17Z

Hi @Choochmeque, can you book a meeting to discuss this? Thanks!

pmaciel

Remarks:

can you add headers to every added source file (.h, .cc, .rs, .toml), similar to the C++ sources (like MIRJob.h and others); we have adopted SPDX headers but haven't transitioned yet
style:
- can you remove lines like //-------------------------...
- two empty lines between: license, headers/pre-processor directives, namespace opening/closing, code propper (class declarations, methods implementation, etc.)
- remove functionality related to mir_tool_call, representation_from (we're transitioning out of this)
- rename the create... methods with make... (this is for consistency with the python bindings)

I don't see that this bindings makes optional use of metkit (but, it's also fine like this.)

It also is disabling tests, which might be part of an implicit contract that these are to be built separately from a main build that actually tests, and stops if tests fail? It is missing tests and/or examples, which are extremely valuable as a high-level documentation or starting points for a newcomer -- specifically the processing of GRIB messages to/from memory are extremely valuable. A unit test and a (small) Jupyter notebook at least?

Note that when I mean "I don't like" or "I prefer" it really is just personal preference and no criticism, just a point of discussion that might sway to a compromise we'd both be happy with!

pmaciel · 2026-08-25T06:38:18Z

@@ -0,0 +1,90 @@
+// mir job bridge — implementation.
+#include "Job.h"


header order should be (separate groups with empty line):

class declaration (correct for this one)

c headers

c++ headers

upstream projects first (eckit, eccodes, atlas, metkit, in this order)

system/platform headers if needed

Always prefix the headers with relative to project root (so the hierarchy is immediately visible)

pmaciel · 2026-08-25T06:42:24Z

@@ -0,0 +1,70 @@
+// mir job bridge — wraps `mir::api::MIRJob`.


These comments aren't necessary (I'm being pedantic)

Fixed in a901981

pmaciel · 2026-08-25T06:44:25Z

+
+//----------------------------------------------------------------------------------------------------------------------
+
+/// A description of the transformation to apply, not the transformation itself:


I prefer long comments using the block version (/* /, or in this case /* */). Since you're refering to reusing MIRJob, also document that the input (MIRInput argument) is consumed with next()

Fixed in edc850b

pmaciel · 2026-08-25T06:48:12Z

+#include <string>
+#include <vector>
+
+namespace mir_bridge {


Discussion only frst - is "bridge" the terminology? I would prefer bindings documentation-wise -- the c++ namespace should really be mir::rust (not bindings not bridge)

"bridge" is just a name - happy to go with mir::rust.

pmaciel · 2026-08-25T06:52:37Z

+
+//----------------------------------------------------------------------------------------------------------------------
+
+/// Static accessors for the mir library itself. Holds no state; it exists as a


This comment says 3 times the same thing

Fixed in a901981

pmaciel · 2026-08-25T07:07:42Z

+/// base — only a subclass can render the contents. The setters shadow inherited
+/// overloads purely so Rust passes `&str` and `&[i64]` instead of building
+/// `CxxString` and `CxxVector` at every call site.
+class Parametrisation final : public mir::param::SimpleParametrisation {


The bindings's Parametrisation are the c++'s SimpleParametrisation. Good change because "Simple" is not a great name and we don't want that to leak. In an eckit module, I refer to this kind as "Custom" (as in, a custom, user-driven parametrisation). What do you think?

Do you mean rename class Parametrisation to Custom? Because SimpleParametrisation will not leak to rust anyway.

pmaciel · 2026-08-25T07:10:42Z

+            "-DCMAKE_BUILD_TYPE={}",
+            bindman_utils::cmake_build_type()
+        ))
+        .arg("-DENABLE_TESTS=OFF")


I don't like this -- why?

cargo builds mir only to link it, and never runs mir's C++ tests, so building them just adds time.

pmaciel · 2026-08-25T07:11:32Z

@@ -0,0 +1,53 @@
+[package]
+name = "mir-sys"
+version = "1.28.2"


Can this be made dynamic? We are already ahead of this, and with a planned 2.0 release this year, or early next year

Fixed in cdfa37b

pmaciel · 2026-08-25T07:12:11Z

+    "eckit-sys/eckit-spec",
+    "eckit-sys/eckit-geo",
+    "eckit-sys/geo-codec-grids",
+    "metkit-sys/vendored",


In our stacks we build metkit after atlas/before mir, maybe this order should generally be reflected here

Fixed in b8dde12

pmaciel · 2026-08-25T07:12:46Z

@@ -0,0 +1,18 @@
+# mir-sys
+
+Low-level Rust bindings to ECMWF's [mir](https://github.com/ecmwf/mir) (Meteorological Interpolation and Regridding) C++ library.


Remove Low-level

Fixed in 69ab838

…per, and Parametrisation classes

…rsion consistency

…ctly

…Output, and Parametrisation files for cleaner code.

…ionally

…sys version to 1.30.0

Choochmeque marked this pull request as ready for review August 24, 2026 11:28

Choochmeque requested a review from pmaciel August 24, 2026 11:28

pmaciel requested changes Aug 25, 2026

View reviewed changes

Choochmeque added 25 commits August 25, 2026 16:31

Initial commit

0900230

Add exception generation for mir's subclasses and eckit's exceptions

146e21a

Refactor C++ bridge for mir library to simplify job and input handling

5a1bbee

Add Rust CI workflow and update dependencies in Cargo.toml

e9b9276

Add type alias for output function and simplify OutputBox definition

d85361d

Update eccodes-sys dependency to new repository for rust bindings

f06bace

Update README to remove reference to planned higher-level mir crate

9b486db

Fix include directive for EckitBridge header file case sensitivity

ff10d38

Reorder include statements for consistency across files

051ffa1

Remove unnecessary comments from C++ header and source files

1bf0a4f

Update documentation comments for Job, MIRInputWrapper, MIROutputWrap…

ceb026e

…per, and Parametrisation classes

Update comments in MirBridge.h

d635cfc

Refactor MIRInput and MIROutput to remove Wrapper classes

5b102d1

Update mir-sys build process to resolve in-tree sources and ensure ve…

d1e4a96

…rsion consistency

Update Cargo.toml and build.rs to include metkit-sys dependency corre…

ccdce71

…ctly

Update README to clarify Rust bindings for ECMWF's mir library

c7b620e

Rename factory methods from 'create' to 'make'

c46f225

Remove unused methods and clean up Job class interface

842a535

Remove unnecessary comment separators from Job, LibMir, MIRInput, MIR…

063826f

…Output, and Parametrisation files for cleaner code.

Add whitespace improvements for better code readability in mir-sys

c496edb

Add copyright and licensing information to Rust project files

a3c0a32

Update dependencies in Cargo.toml to use latest branches and revisions

69d55ae

Add support for docs.rs builds by generating exception headers condit…

b4a417b

…ionally

Remove unnecessary build configuration options in build.rs

d55c906

Update Rust CI workflow to include documentation checks and bump mir-…

0f1e9bb

…sys version to 1.30.0

Choochmeque force-pushed the rust-bindings branch from b08baad to 0f1e9bb Compare August 25, 2026 14:52

		@@ -0,0 +1,90 @@
		// mir job bridge — implementation.
		#include "Job.h"

		@@ -0,0 +1,70 @@
		// mir job bridge — wraps `mir::api::MIRJob`.


		//----------------------------------------------------------------------------------------------------------------------

		/// A description of the transformation to apply, not the transformation itself:


		//----------------------------------------------------------------------------------------------------------------------

		/// Static accessors for the mir library itself. Holds no state; it exists as a

		@@ -0,0 +1,18 @@
		# mir-sys

		Low-level Rust bindings to ECMWF's [mir](https://github.com/ecmwf/mir) (Meteorological Interpolation and Regridding) C++ library.

Uh oh!

Conversation

Choochmeque commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Declaration

Uh oh!

codecov-commenter commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pmaciel commented Jun 21, 2026

Uh oh!

pmaciel 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!

Choose a reason for hiding this comment

Uh oh!

Choochmeque Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Choochmeque commented May 6, 2026 •

edited

Loading

codecov-commenter commented May 6, 2026 •

edited

Loading

Choochmeque Aug 25, 2026 •

edited

Loading