[wdeconinck]

This pull request tries to address #39 , by standardizing and improving the __repr__ string representations for many core classes, making debugging and interactive usage more informative and user-friendly. It also adds a new test to ensure that the __repr__ output for point classes is round-trip safe (i.e., objects can be reconstructed from their string representation).

Key changes include:

Improved and Standardized __repr__ Methods:

• Added or updated __repr__ implementations for numerous classes (e.g., Field, Mesh, StructuredMeshGenerator, IrregularConnectivity, BlockConnectivity, MultiBlockConnectivity, Nodes, HybridElements, FunctionSpace, EdgeColumns, NodeColumns, CellColumns, Topology, and Gmsh). These representations now follow a consistent, Pythonic format (e.g., <atlas4py.ClassName ...>), often including key attributes for easier inspection.

Testing Improvements:

• Added new tests to verify that the __repr__ methods return the expected strings for all major classes, including round-trip tests for point types and coverage for mesh/connectivity/function space objects.

Code Quality and Consistency:

• Ensured all classes now have a meaningful and consistent string representation, improving usability in interactive sessions and debugging.

[Copilot]

🟡 Changes recommended

Several representations reference the nonexistent atlas4py.mesh module, and most new outputs remain untested.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves atlas4py object representations for clearer debugging and interactive use.

Changes:

• Standardizes representations across core bindings.
• Makes point representations round-trip safe.
• Adds parameterized point representation tests.

File summaries

File	Description
src/atlas4py/_atlas4py.cpp	Adds and updates binding representations.
tests/test_bindings.py	Tests point representation round trips.

Review details

Suppressed comments (7)

src/atlas4py/_atlas4py.cpp:387

• StructuredMeshGenerator is exported from the root package, and there is no atlas4py.mesh submodule. Use its actual public path so the new representation does not advertise a nonexistent symbol.

            return "<atlas4py.mesh.StructuredMeshGenerator>";

src/atlas4py/_atlas4py.cpp:414

• Both branches identify this root-level binding as atlas4py.mesh.IrregularConnectivity, but mesh is not a Python submodule. The representation should use the class's exported path, atlas4py.IrregularConnectivity.

        .def("__repr__", []( mesh::IrregularConnectivity const& c ) {

src/atlas4py/_atlas4py.cpp:433

• Both branches use the nonexistent path atlas4py.mesh.BlockConnectivity; this class is registered on the root module and exposed as atlas4py.BlockConnectivity.

        .def("__repr__", []( mesh::BlockConnectivity const& c ) {

src/atlas4py/_atlas4py.cpp:461

• The representation points to atlas4py.mesh.MultiBlockConnectivity, although no mesh Python submodule exists and this binding is exported at atlas4py.MultiBlockConnectivity.

        .def("__repr__", []( mesh::MultiBlockConnectivity const& c ) {

src/atlas4py/_atlas4py.cpp:481

• Nodes is a root-level Python binding, not a member of an atlas4py.mesh submodule. This should show atlas4py.Nodes to avoid reporting a nonexistent public path.

            oss << "<atlas4py.mesh.Nodes size=" << n.size() << ">";

src/atlas4py/_atlas4py.cpp:496

• This class is exported as atlas4py.HybridElements; atlas4py.mesh.HybridElements is not available because no mesh submodule is created.

            oss << "<atlas4py.mesh.HybridElements size=" << he.size() << ">";

src/atlas4py/_atlas4py.cpp:600

• Topology is registered directly on the root module, so atlas4py.mesh.Nodes.Topology is not a valid Python path. Report its actual exported name, atlas4py.Topology.

        return "<atlas4py.mesh.Nodes.Topology>";

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Point representations fail round trips for non-finite coordinates, and the new stream usage lacks its required header.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/atlas4py/_atlas4py.cpp:234

• This is not round-trip safe for all accepted double values: Python renders non-finite values as bare inf, -inf, or nan, so evaluating the resulting point representation raises NameError. Format non-finite coordinates as evaluable expressions (for example, float('inf')) and cover them in the round-trip test.

This issue also appears on line 241 of the same file.

src/atlas4py/_atlas4py.cpp:241

• PointXY has the same round-trip failure for accepted non-finite coordinates: nb::str(nb::float_(...)) emits bare inf/nan, which eval cannot resolve. Use an evaluable representation for special floating-point values and add corresponding test cases.

            return "atlas4py.PointXY(x="_s + nb::str( nb::float_( p.x() ) ) + ", y="_s + nb::str( nb::float_( p.y() ) ) + ")"_s;

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[tehrengruber]

Looks good. It might make sense to use __repr__ of properties that are atlas4py properties themselves in the future, but right now most of the things appear to be primitive types (e.g. int) anyway. Further one could likely write a function generate_repr(class_name, attr1, attr2) or similar using some template that automatically generates the function (each attr would be first converted into a python object on which one could then call __repr__).

[tehrengruber-ai]

Solid, well-tested cleanup overall — the nb::float_ switch for the point types is the right fix for round-trip precision, and the connectivity/mesh reprs are genuinely useful for interactive work. A few things below, one of which means the Topology part of the change doesn't actually take effect.

Also checked and found fine: the size_t overload for metadata().get("halo", ...) exists in eckit and atlas stores halo as a non-negative int; build_edges(mesh) works single-arg thanks to the "config"_a = util::Config() default; str(float) in Python has been the shortest round-trip repr since 3.1, so the precision intent holds; and the stale _atlas4py.Grid(...) strings in examples/Atlas4Py-Test.ipynb are saved notebook outputs that CI never executes.

[tehrengruber-ai]

This __repr__ is unreachable — it can never be called.

mesh::Nodes::Topology is atlas::util::Topology, a static-only flag holder (class Topology : public util::Bitflags with nothing but an anonymous enum). The binding above declares it as nb::class_<mesh::Nodes::Topology> topology(m, "Topology") with no nb::init, and no other binding in this file returns or casts a Topology instance. So there is no way to obtain a Topology object from Python, and this instance method never runs.

repr(atlas4py.Topology) resolves through the metaclass rather than this method, so it still prints <class 'atlas4py.Topology'> — unchanged by this PR.

The tests corroborate it: every other new __repr__ here gets an assertion, and this is the only one that doesn't. Since the PR description lists Topology among the classes whose representation was improved, this is worth resolving either way — drop it, or if a nicer class-level repr is the goal, that needs a different mechanism (a custom metaclass), not .def.

[tehrengruber-ai]

The name is emitted unquoted, which degrades badly for unnamed fields.

create_field takes name as optional ("name"_a = std::nullopt), and atlas's FieldImpl constructor always calls rename(name) — so a field created without a name has name() == "" rather than throwing. The repr then reads:

<atlas4py.Field name= shape=(380, 1) dtype=float64>

Names containing spaces are ambiguous in the same way. Quoting the value fixes both and matches what the neighbouring Metadata/Config reprs already do for strings:

<< " name='" << field.name() << "'"

[tehrengruber-ai]

Minor consistency: EdgeColumns reports ec.size() here while NodeColumns and CellColumns below use nb_nodes() / nb_cells(). All three are equivalent in atlas (size() just returns the respective count), so it's cosmetic — but worth picking one spelling across the three.

Separately, the invalid branch just above (and its counterparts in NodeColumns/CellColumns) isn't exercised by any of the new tests.

[tehrengruber-ai]

Nit: trailing whitespace after << ">";. No CI job enforces formatting today, but the repo does ship a .clang-format.

[tehrengruber-ai]

Coverage gap worth closing while you're here: this PR also reformats the Projection, Domain, Grid, and Spacing reprs from _atlas4py.X(...) to <atlas4py.X(...)>, but none of those has an assertion anywhere in the suite — the new tests only cover the reprs this PR newly adds. A repr(structured_grid) assertion would cover the one users hit most often.

On the flip side, the hardcoded counts here (nb_edges=17, and 799/420/380 below) and the exact Metadata dict key ordering will need touching on atlas upgrades. That matches the existing test_config_repr_roundtrip style, so I'd leave it as-is — just flagging the maintenance cost.