From 3644b68f3fc2a2653316e21b90b4789e8bd43604 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Wed, 11 Feb 2026 17:37:37 +0100 Subject: [PATCH 01/14] Improve config --- src/atlas4py/CMakeLists.txt | 5 +- src/atlas4py/_atlas4py.cpp | 147 ++--------------- src/atlas4py/_atlas4py_Config.cpp | 262 ++++++++++++++++++++++++++++++ src/atlas4py/_atlas4py_Config.hpp | 21 +++ tests/test_bindings.py | 155 +++++++++++++++++- 5 files changed, 451 insertions(+), 139 deletions(-) create mode 100644 src/atlas4py/_atlas4py_Config.cpp create mode 100644 src/atlas4py/_atlas4py_Config.hpp diff --git a/src/atlas4py/CMakeLists.txt b/src/atlas4py/CMakeLists.txt index 90c1963..c0fa4ac 100644 --- a/src/atlas4py/CMakeLists.txt +++ b/src/atlas4py/CMakeLists.txt @@ -81,7 +81,10 @@ endif() ### Python bindings module atlas4py -nanobind_add_module(_atlas4py _atlas4py.cpp) +nanobind_add_module(_atlas4py + _atlas4py.cpp + _atlas4py_Config.cpp +) target_link_libraries(_atlas4py PUBLIC atlas) target_compile_definitions(_atlas4py PRIVATE ATLAS4PY_VERSION_STRING=${PROJECT_VERSION_FULL}) diff --git a/src/atlas4py/_atlas4py.cpp b/src/atlas4py/_atlas4py.cpp index 37ba16a..aa9adad 100644 --- a/src/atlas4py/_atlas4py.cpp +++ b/src/atlas4py/_atlas4py.cpp @@ -40,6 +40,8 @@ namespace pluto { #include "eckit/value/Value.h" #include "eckit/config/Configuration.h" +#include "_atlas4py_Config.hpp" + namespace nb = ::nanobind; using namespace atlas; @@ -151,77 +153,6 @@ void atlasInitialise() { // the atlas library and deleting it may cause issues. } -nb::object toPyObject( eckit::Configuration const& v ); -nb::object toPyObject( eckit::Configuration const& v, std::string const& key ); - -nb::object toPyObject(bool v) { - return nb::bool_(v); -} -nb::object toPyObject(long v) { - return nb::int_(v); -} -nb::object toPyObject(double v) { - return nb::float_(v); -} -nb::object toPyObject(std::string const& v) { - return nb::str(v.c_str()); -} -template -nb::object toPyObject( std::vector const& v ) { - nb::list ret; - for ( auto const& val : v ) { - ret.append( toPyObject( val ) ); - } - return ret; -} - -nb::object toPyObject( eckit::Configuration const& v, std::string const& key ) { - if ( v.isSubConfiguration ( key ) ) { - return toPyObject( v.getSubConfiguration( key ) ); - } - else if (v.isBoolean( key )) { - return toPyObject( v.getBool( key ) ); - } - else if (v.isIntegral( key )) { - return toPyObject( v.getLong( key ) ); - } - else if (v.isFloatingPoint( key )) { - return toPyObject( v.getDouble( key ) ); - } - else if (v.isString( key )) { - return toPyObject( v.getString( key ) ); - } - else if (v.isSubConfigurationList( key )) { - std::vector subconfigs = v.getSubConfigurations( key ); - return toPyObject( subconfigs ); - } - else if (v.isIntegralList( key )) { - std::vector values = v.getLongVector( key ); - return toPyObject( values ); - } - else if (v.isFloatingPointList( key )) { - std::vector values = v.getDoubleVector( key ); - return toPyObject( values ); - } - else if (v.isStringList( key )) { - std::vector values = v.getStringVector( key ); - return toPyObject( values ); - } - else if (v.isBooleanList( key )) { - throw std::out_of_range( "boolean lists not supported for key " + key ); - } - else { - throw std::out_of_range( "type of value unsupported for key " + key ); - } -} - -nb::object toPyObject( eckit::Configuration const& v ) { - nb::dict ret; - for ( auto const& key : v.keys()) { - ret[ key.c_str() ] = toPyObject( v, key ); - } - return ret; -} int get_nb_device_type( const void* ptr ) { if (pluto::is_pinned(ptr)) { @@ -293,6 +224,8 @@ NB_MODULE( _atlas4py, m ) { .def("_finalise", atlas::finalise); m.attr("__version__") = STRINGIFY(ATLAS4PY_VERSION_STRING); + atlas4py::bind_Config( m ); + nb::class_( m, "PointLonLat" ) .def( nb::init(), "lon"_a, "lat"_a ) .def_prop_ro( "lon", nb::overload_cast<>( &PointLonLat::lon, nb::const_ ) ) @@ -310,7 +243,7 @@ NB_MODULE( _atlas4py, m ) { nb::class_( m, "Projection" ) .def( "__repr__", []( Projection const& p ) { - return "_atlas4py.Projection("_s + nb::str( toPyObject( p.spec() ) ) + ")"_s; + return "_atlas4py.Projection("_s + nb::str( atlas4py::make_object( p.spec() ) ) + ")"_s; } ); nb::class_( m, "Domain" ) @@ -319,7 +252,7 @@ NB_MODULE( _atlas4py, m ) { .def_prop_ro( "units", &Domain::units ) .def( "__repr__", []( Domain const& d ) { if (d) { - return nb::str("_atlas4py.Domain("_s + nb::str( toPyObject( d.spec() ) ) + ")"_s); + return nb::str("_atlas4py.Domain("_s + nb::str( atlas4py::make_object( d.spec() ) ) + ")"_s); } return nb::str("_atlas4py.Domain()"_s); } ); @@ -334,13 +267,13 @@ NB_MODULE( _atlas4py, m ) { .def_prop_ro( "projection", &Grid::projection ) .def_prop_ro( "domain", &Grid::domain ) .def( "__repr__", - []( Grid const& g ) { return "_atlas4py.Grid("_s + nb::str( toPyObject( g.spec() ) ) + ")"_s; } ); + []( Grid const& g ) { return "_atlas4py.Grid("_s + nb::str( atlas4py::make_object( g.spec() ) ) + ")"_s; } ); nb::class_( m, "Spacing" ) .def( "__len__", &grid::Spacing::size ) .def( "__getitem__", &grid::Spacing::operator[]) .def( "__repr__", []( grid::Spacing const& spacing ) { - return "_atlas4py.Spacing("_s + nb::str( toPyObject( spacing.spec() ) ) + ")"_s; + return "_atlas4py.Spacing("_s + nb::str( atlas4py::make_object( spacing.spec() ) ) + ")"_s; } ); nb::class_( m, "LinearSpacing" ) .def( nb::init(), "start"_a, "stop"_a, "N"_a, "endpoint_included"_a = true ); @@ -373,41 +306,6 @@ NB_MODULE( _atlas4py, m ) { .def_prop_ro( "regular", &StructuredGrid::regular ) .def_prop_ro( "periodic", &StructuredGrid::periodic ); - nb::class_( m, "eckit.Configuration" ); - - nb::class_( m, "eckit.LocalConfiguration" ); - - // TODO This is a duplicate of metadata below (because same base class) - nb::class_( m, "Config" ) - .def( nb::init() ) - .def( "__setitem__", - []( util::Config& config, std::string const& key, nb::object value ) { - if ( nb::isinstance( value ) ) - config.set( key, nb::cast( value ) ); - else if ( nb::isinstance( value ) ) - config.set( key, nb::cast( value ) ); - else if ( nb::isinstance( value ) ) - config.set( key, nb::cast( value ) ); - else if ( nb::isinstance( value ) ) - config.set( key, nb::cast( value ) ); - else - throw std::out_of_range( "type of value unsupported" ); - } ) - .def( "__getitem__", - []( util::Config& config, std::string const& key ) -> nb::object { - if ( !config.has( key ) ) - throw std::out_of_range( "key <" + key + "> could not be found" ); - - // TODO: We have to query metadata.get() even though this should - // not be done (see comment in Config::get). We cannot - // avoid this right now because otherwise we cannot query - // the type of the underlying data. - return toPyObject( config, key ); - } ) - .def( "__repr__", []( util::Config const& config ) { - return "_atlas4py.Config("_s + nb::str( toPyObject( config ) ) + ")"_s; - } ); - nb::class_( m, "Field" ) .def_prop_ro( "name", &Field::name ) .def_prop_ro( "strides", &Field::strides ) @@ -574,34 +472,9 @@ NB_MODULE( _atlas4py, m ) { .def_prop_ro( "cells", &functionspace::CellColumns::cells ) .def_prop_ro( "valid", &functionspace::CellColumns::valid ); - nb::class_( m, "Metadata" ) - .def_prop_ro( "keys", &util::Metadata::keys ) - .def( "__setitem__", - []( util::Metadata& metadata, std::string const& key, nb::object value ) { - if ( nb::isinstance( value ) ) - metadata.set( key, nb::cast(value) ); - else if ( nb::isinstance( value ) ) - metadata.set( key, nb::cast(value) ); - else if ( nb::isinstance( value ) ) - metadata.set( key, nb::cast(value) ); - else if ( nb::isinstance( value ) ) - metadata.set( key, nb::cast(value) ); - else - throw std::out_of_range( "type of value unsupported" ); - } ) - .def( "__getitem__", - []( util::Metadata& metadata, std::string const& key ) -> nb::object { - if ( !metadata.has( key ) ) - throw std::out_of_range( "key <" + key + "> could not be found" ); - - // TODO: We have to query metadata.get() even though this should - // not be done (see comment in Config::get). We cannot - // avoid this right now because otherwise we cannot query - // the type of the underlying data. - return toPyObject( metadata, key ); - } ) + nb::class_( m, "Metadata" ) .def( "__repr__", []( util::Metadata const& metadata ) { - return "_atlas4py.Metadata("_s + nb::str( toPyObject( metadata ) ) + ")"_s; + return "_atlas4py.Metadata("_s + nb::str( atlas4py::make_object( metadata ) ) + ")"_s; } ); nb::class_ topology( m, "Topology" ); diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp new file mode 100644 index 0000000..f40fd81 --- /dev/null +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -0,0 +1,262 @@ +#include "_atlas4py_Config.hpp" + +#include +#include +#include + +#include +#include +#include + +#include "eckit/config/Configuration.h" +#include "eckit/config/YAMLConfiguration.h" +#include "eckit/filesystem/PathName.h" + +#include "atlas/util/Config.h" + +namespace nb = ::nanobind; + +namespace { + +nb::object _toPyObject( eckit::Configuration const& v ); +nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ); + +nb::object _toPyObject(bool v) { + return nb::bool_(v); +} +nb::object _toPyObject(long v) { + return nb::int_(v); +} +nb::object _toPyObject(double v) { + return nb::float_(v); +} +nb::object _toPyObject(std::string const& v) { + return nb::str(v.c_str()); +} +template +nb::object _toPyObject( std::vector const& v ) { + nb::list ret; + for ( auto const& val : v ) { + ret.append( _toPyObject( val ) ); + } + return ret; +} + +template <> +nb::object _toPyObject( std::vector const& v ) { + nb::list ret; + for ( auto const& val : v ) { + ret.append( _toPyObject( val ) ); + } + return ret; +} + +nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ) { + if ( v.isSubConfiguration ( key ) ) { + return _toPyObject( v.getSubConfiguration( key ) ); + } + else if (v.isBoolean( key )) { + return _toPyObject( v.getBool( key ) ); + } + else if (v.isIntegral( key )) { + return _toPyObject( v.getLong( key ) ); + } + else if (v.isFloatingPoint( key )) { + return _toPyObject( v.getDouble( key ) ); + } + else if (v.isString( key )) { + return _toPyObject( v.getString( key ) ); + } + else if (v.isSubConfigurationList( key )) { + std::vector subconfigs = v.getSubConfigurations( key ); + return _toPyObject( subconfigs ); + } + else if (v.isBooleanList( key )) { + auto getBooleanVector = [&]() { + std::vector values = v.getLongVector( key ); + std::vector bools; + bools.reserve( values.size() ); + for ( auto const& val : values ) { + bools.push_back( static_cast( val ) ); + } + return bools; + }; + return _toPyObject( getBooleanVector() ); + } + else if (v.isIntegralList( key )) { + std::vector values = v.getLongVector( key ); + return _toPyObject( values ); + } + else if (v.isFloatingPointList( key )) { + std::vector values = v.getDoubleVector( key ); + return _toPyObject( values ); + } + else if (v.isStringList( key )) { + std::vector values = v.getStringVector( key ); + return _toPyObject( values ); + } + else { + throw std::out_of_range( "type of value unsupported for key " + key ); + } +} + +nb::object _toPyObject( eckit::Configuration const& v ) { + nb::dict ret; + for ( auto const& key : v.keys()) { + ret[ key.c_str() ] = _toPyObject( v, key ); + } + return ret; +} + + +void config_set( eckit::LocalConfiguration& config, const std::string& key, nb::handle value ) { + auto py_type_str = [](nb::handle h) -> std::string { + nb::object type_obj = nb::borrow(Py_TYPE(h.ptr())); + nb::object name = type_obj.attr("__name__"); + return nb::cast(name); + }; + + if (nb::isinstance(value)) { + config.set(key, nb::cast(value)); + } else if (nb::isinstance(value)) { + config.set(key, nb::cast(value)); + } else if (nb::isinstance(value)) { + config.set(key, nb::cast(value)); + } else if (nb::isinstance(value)) { + config.set(key, nb::cast(value)); + } else if (nb::isinstance(value)) { + nb::object seq = nb::cast(value); + size_t n = nb::len(seq); + if (n == 0) { + config.set(key, std::vector{}); + return; + } + auto elem = seq[0]; + auto handle_sequence = [&](auto&& conv) { + using VecType = std::vector>; + VecType vec; + for (size_t i = 0; i < n; ++i) vec.push_back(conv(seq[i])); + config.set(key, vec); + }; + if (nb::isinstance(elem)) { + std::vector vec; + for (size_t i = 0; i < n; ++i) vec.push_back( nb::cast(seq[i]) ? 1 : 0 ); + config.set(key, vec); + } else if (nb::isinstance(elem)) { + handle_sequence([](nb::handle v) { return nb::cast(v); }); + } else if (nb::isinstance(elem)) { + handle_sequence([](nb::handle v) { return nb::cast(v); }); + } else if (nb::isinstance(elem)) { + handle_sequence([](nb::handle v) { return nb::cast(v); }); + } else if (nb::isinstance(elem)) { + std::vector vec; + for (size_t i = 0; i < n; ++i) { + eckit::LocalConfiguration subconfig; + nb::object mapping = nb::cast(seq[i]); + for (nb::handle item : mapping.attr("keys")()) { + config_set(subconfig, nb::cast(item), mapping[item]); + } + vec.push_back(subconfig); + } + config.set(key, vec); + } else { + throw std::out_of_range("Unsupported sequence element type for key '" + key + "': got type '" + py_type_str(elem) + "'"); + } + } else if (nb::isinstance(value)) { + eckit::LocalConfiguration subconfig; + nb::object mapping = nb::cast(value); + for (nb::handle item : mapping.attr("keys")()) { + config_set(subconfig, nb::cast(item), mapping[item]); + } + config.set(key, subconfig); + } else { + throw std::out_of_range("type of value unsupported for key '" + key + "': got type '" + py_type_str(value) + "'"); + } +} + +std::string to_lowercase(const std::string& str) { + std::string lowercase_str = str; + std::transform(lowercase_str.begin(), lowercase_str.end(), lowercase_str.begin(), + [](unsigned char c) { return std::tolower(c); }); + return lowercase_str; +} + +} // namespace + +nb::object atlas4py::make_object( eckit::Configuration const& v ) { + return _toPyObject( v ); +} + + +atlas::util::Config atlas4py::make_Config( nb::kwargs const& kwargs ) { + atlas::util::Config config; + for( const auto& pair : kwargs ) { + const auto key = nb::cast(pair.first); + const auto& value = pair.second; + config_set(config, key, value); + } + return config; +} + +void atlas4py::bind_Config( nb::module_& m ) { + using namespace nanobind::literals; + + nb::class_( m, "eckit.Configuration" ) + .def( "keys", &eckit::Configuration::keys ) + .def( "__contains__", + []( eckit::Configuration const& config, std::string const& key ) { + return config.has( key ); + } ) + .def( "__getitem__", + []( eckit::Configuration const& config, std::string const& key ) -> nb::object { + if ( !config.has( key ) ) + throw std::out_of_range( "key <" + key + "> could not be found" ); + return _toPyObject( config, key ); + } ) + .def( "__iter__", + []( eckit::Configuration const& config ) { + return nb::iter(nb::cast(config.keys())); + } ) + .def( "__len__", + []( eckit::Configuration const& config ) { + return config.keys().size(); + } ) + .def( "__repr__", []( eckit::Configuration const& config ) { + return "_atlas4py.eckit.Configuration("_s + nb::str( make_object( config ) ) + ")"_s; + } ); + + nb::class_( m, "eckit.LocalConfiguration" ) + .def( nb::init() ) + .def( "__setitem__", + []( eckit::LocalConfiguration& config, std::string const& key, nb::object value ) { + config_set(config,key,value); + } ) + .def( "__repr__", []( eckit::LocalConfiguration const& config ) { + return "_atlas4py.eckit.LocalConfiguration("_s + nb::str( make_object( config ) ) + ")"_s; + } ); + + nb::class_( m, "Config" ) + .def( nb::init() ) + .def_static( "from_kwargs", []( nb::kwargs kwargs ) { + return make_Config(kwargs); + } ) + .def_static( "from_yaml", []( std::string const& yaml ) { + return atlas::util::Config( eckit::YAMLConfiguration(yaml) ); + }, "yaml"_a ) + .def_static( "from_json", []( std::string const& json ) { + return atlas::util::Config( eckit::YAMLConfiguration(json) ); + }, "json"_a ) + .def_static( "from_file", []( const nb::object path, std::string const& format ) { + // path accepts string but also path-like objects (e.g. pathlib.Path) + if ( !format.empty() ) { + auto format_lowercase = to_lowercase(format); + if (format_lowercase != "yaml" && format_lowercase != "json") { + throw std::runtime_error("Only 'yaml' or 'json' format is supported"); + } + } + return atlas::util::Config( eckit::PathName{nb::cast( nb::str(path) )} ); + }, "path"_a, "format"_a = "" ) + .def( "__repr__", []( atlas::util::Config const& config ) { + return "_atlas4py.Config("_s + nb::str( make_object( config ) ) + ")"_s; + } ); +} diff --git a/src/atlas4py/_atlas4py_Config.hpp b/src/atlas4py/_atlas4py_Config.hpp new file mode 100644 index 0000000..82f2d66 --- /dev/null +++ b/src/atlas4py/_atlas4py_Config.hpp @@ -0,0 +1,21 @@ +#pragma once + +#include + +#include "eckit/config/Configuration.h" +#include "atlas/util/Config.h" + +namespace nb = ::nanobind; + +namespace atlas4py { + + // Bindings for atlas::util::Config + void bind_Config( nb::module_& m ); + + // Convert eckit::Configuration to a corresponding Python object + nb::object make_object( eckit::Configuration const& v ); + + // Create an atlas::util::Config from Python keyword arguments + atlas::util::Config make_Config( nb::kwargs const& kwargs ); + +} // namespace atlas4py diff --git a/tests/test_bindings.py b/tests/test_bindings.py index ee3282a..ba314fb 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -1,7 +1,7 @@ +import atlas4py import numpy as np import pytest -import atlas4py # -- Fixtures -- @@ -162,3 +162,156 @@ def test_gmsh_output(structured_mesh): # Clean up the output file os.remove(output_file) + +def test_config_set_python_types(): + from atlas4py import Config + + # Flat types + c = Config() + c["int"] = 42 + c["float"] = 3.14 + c["bool"] = True + c["str"] = "hello" + c["int_list"] = [1, 2, 3] + c["float_list"] = [1.1, 2.2, 3.3] + c["str_list"] = ["a", "b", "c"] + c["bool_list"] = [True, False, True] + + # Nested dict (mapping protocol) + c["nested"] = {"x": 1, "y": 2} + # List of dicts (sequence of mappings) + c["list_of_dicts"] = [{"a": 1}, {"b": 2}] + + # Retrieve and check + assert c["int"] == 42 + assert c["float"] == pytest.approx(3.14) + assert c["bool"] is True + assert c["str"] == "hello" + assert c["int_list"] == [1, 2, 3] + assert c["float_list"] == pytest.approx([1.1, 2.2, 3.3]) + assert c["str_list"] == ["a", "b", "c"] + assert c["bool_list"] == [True, False, True] + assert dict(c["nested"]) == {"x": 1, "y": 2} + assert [dict(d) for d in c["list_of_dicts"]] == [{"a": 1}, {"b": 2}] + +def test_config_contains(): + config = atlas4py.Config.from_kwargs(option1="value1", option2=42) + assert "option1" in config + assert "option2" in config + assert "option3" not in config + # dotted path lookup + config["outer.inner"] = 1 + assert "outer" in config + assert "outer.inner" in config + assert "outer.missing" not in config + + +def test_config_mapping_protocol(): + config = atlas4py.Config.from_kwargs(option1="value1", option2=42, option3=3.14) + # dict() uses keys() + __getitem__ + d = dict(config) + assert d == {"option1": "value1", "option2": 42, "option3": 3.14} + # __iter__ + assert list(config) == config.keys() + # __len__ + assert len(config) == 3 + + +def test_config_from_kwargs(): + config = atlas4py.Config.from_kwargs(option1="value1", option2=42) + config["option3"] = 3.14 + assert config["option1"] == "value1" + assert config["option2"] == 42 + assert config["option3"] == 3.14 + assert config.keys() == ["option1", "option2", "option3"] + +def test_config_from_yaml(): + yaml_string = """ +option1: value1 +option2: 42 +outer: + inner_option1: 3.14 + inner_option2: true +""" + config = atlas4py.Config.from_yaml(yaml_string) + config["outer.inner_option3"] = "added_value" + + assert config.keys() == ["option1", "option2", "outer"] + assert config["option1"] == "value1" + assert config["option2"] == 42 + assert config["outer.inner_option1"] == 3.14 + assert config["outer.inner_option2"] is True + outer = config["outer"] + assert outer["inner_option1"] == 3.14 + assert outer["inner_option2"] is True + assert outer["inner_option3"] == "added_value" + + +def test_config_boolean_list_roundtrip_from_yaml(): + yaml_string = """ +flags: + - true + - false + - true +""" + config = atlas4py.Config.from_yaml(yaml_string) + + flags = config["flags"] + assert flags == [True, False, True] + assert all(isinstance(flag, bool) for flag in flags) + + +def test_config_boolean_list_roundtrip_from_json(tmp_path): + test_config_json = tmp_path / "test_config.json" + test_config_json.write_text( + """ +{ + "flags": [true, false, true] +} +""" + ) + + config = atlas4py.Config.from_file(test_config_json, format="json") + + flags = config["flags"] + assert flags == [True, False, True] + assert all(isinstance(flag, bool) for flag in flags) + + +def test_config_boolean_list_roundtrip_from_json_string(): + json_string = """ +{ + "flags": [true, false, true] +} +""" + config = atlas4py.Config.from_json(json_string) + + flags = config["flags"] + assert flags == [True, False, True] + assert all(isinstance(flag, bool) for flag in flags) + + +def test_config_from_file(tmp_path): + test_config_yaml = tmp_path / "test_config.yaml" + with open(test_config_yaml, "w") as f: + f.write( + """ +option1: value1 +option2: 42 +outer: + inner_option1: 3.14 + inner_option2: true +""" + ) + config = atlas4py.Config.from_file(test_config_yaml) + config["outer.inner_option3"] = "added_value" + + assert config.keys() == ["option1", "option2", "outer"] + assert config["option1"] == "value1" + assert config["option2"] == 42 + assert config["outer.inner_option1"] == 3.14 + assert config["outer.inner_option2"] is True + outer = config["outer"] + assert outer["inner_option1"] == 3.14 + assert outer["inner_option2"] is True + assert outer["inner_option3"] == "added_value" From df967b12f80df323735c2d6664b9994015092c69 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Wed, 1 Jul 2026 10:41:22 +0200 Subject: [PATCH 02/14] Version 0.41.1.dev5 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index cdd6a82..ba11f16 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ dependencies = [] description = 'Python bindings for Atlas: a ECMWF library for parallel data-structures' name = 'atlas4py' -version = '0.41.1.dev4' # ...dev : ...dev +version = '0.41.1.dev5' # ...dev : ...dev license = {text = "Apache License 2.0"} readme = {file = 'README.md', content-type = 'text/markdown'} authors = [{email = 'willem.deconinck@ecmwf.int'}, {name = 'Willem Deconinck'}] From 87c8da373d9aadef4891c5c3f13df22fc9635ab4 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Fri, 28 Aug 2026 16:21:35 +0200 Subject: [PATCH 03/14] Fix nit on using nb::str --- src/atlas4py/_atlas4py_Config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index f40fd81..af840d0 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -31,7 +31,7 @@ nb::object _toPyObject(double v) { return nb::float_(v); } nb::object _toPyObject(std::string const& v) { - return nb::str(v.c_str()); + return nb::str(v.c_str(), v.size()); } template nb::object _toPyObject( std::vector const& v ) { From 3cf4e1fe0e6805f37b89f20b64062cdb0685e6df Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Fri, 28 Aug 2026 16:27:33 +0200 Subject: [PATCH 04/14] Remove unneeded overload --- src/atlas4py/_atlas4py_Config.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index af840d0..f9f2c2a 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -42,14 +42,6 @@ nb::object _toPyObject( std::vector const& v ) { return ret; } -template <> -nb::object _toPyObject( std::vector const& v ) { - nb::list ret; - for ( auto const& val : v ) { - ret.append( _toPyObject( val ) ); - } - return ret; -} nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ) { if ( v.isSubConfiguration ( key ) ) { From 917a370926611f65f2e1565180e12980dc7a9457 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Fri, 28 Aug 2026 16:28:09 +0200 Subject: [PATCH 05/14] Forward compatibility to handle vector correctly. --- src/atlas4py/_atlas4py_Config.cpp | 81 ++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 13 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index f9f2c2a..4e14b3e 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include #include @@ -18,6 +20,57 @@ namespace nb = ::nanobind; namespace { +// getBoolVector() and set(..., std::vector) will be available in a future eckit version. +template +struct has_bool_vector_set : std::false_type {}; + +template +struct has_bool_vector_set< + T, + std::void_t().set( std::declval(), std::declval const&>() ) )>> + : std::is_same< + decltype( std::declval().set( std::declval(), std::declval const&>() ) ), + T&> {}; + +template +inline constexpr bool has_bool_vector_set_v = has_bool_vector_set>::value; + +template +struct has_getBoolVector : std::false_type {}; + +template +struct has_getBoolVector< + T, + std::void_t().getBoolVector( std::declval() ) )>> + : std::is_same< + decltype( std::declval().getBoolVector( std::declval() ) ), + std::vector> {}; + +template +inline constexpr bool has_getBoolVector_v = has_getBoolVector>::value; + +template +std::vector get_bool_vector_fallback( ConfigurationT const& config, std::string const& key ) { + std::vector integers = config.getLongVector( key ); + std::vector values; + values.reserve( integers.size() ); + for ( auto const& integer_value : integers ) { + values.push_back( static_cast( integer_value ) ); + } + return values; +} + +template +void set_bool_vector_fallback( ConfigurationT& config, std::string const& key, + std::vector const& values ) { + std::vector integers; + integers.reserve( values.size() ); + for ( bool value : values ) { + integers.push_back( value ? 1 : 0 ); + } + config.set( key, integers ); +} + nb::object _toPyObject( eckit::Configuration const& v ); nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ); @@ -64,16 +117,15 @@ nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ) return _toPyObject( subconfigs ); } else if (v.isBooleanList( key )) { - auto getBooleanVector = [&]() { - std::vector values = v.getLongVector( key ); - std::vector bools; - bools.reserve( values.size() ); - for ( auto const& val : values ) { - bools.push_back( static_cast( val ) ); + auto get_bool_vector = [&](auto const& config) { + if constexpr (has_getBoolVector_v) { + return config.getBoolVector( key ); + } + else { + return get_bool_vector_fallback( config, key ); } - return bools; }; - return _toPyObject( getBooleanVector() ); + return _toPyObject( get_bool_vector( v ) ); } else if (v.isIntegralList( key )) { std::vector values = v.getLongVector( key ); @@ -125,15 +177,18 @@ void config_set( eckit::LocalConfiguration& config, const std::string& key, nb:: } auto elem = seq[0]; auto handle_sequence = [&](auto&& conv) { - using VecType = std::vector>; + using ValueType = std::decay_t; + using VecType = std::vector; VecType vec; for (size_t i = 0; i < n; ++i) vec.push_back(conv(seq[i])); - config.set(key, vec); + if constexpr (std::is_same_v && !has_bool_vector_set_v) { + set_bool_vector_fallback( config, key, vec ); + } else { + config.set(key, vec); + } }; if (nb::isinstance(elem)) { - std::vector vec; - for (size_t i = 0; i < n; ++i) vec.push_back( nb::cast(seq[i]) ? 1 : 0 ); - config.set(key, vec); + handle_sequence([](nb::handle v) { return nb::cast(v); }); } else if (nb::isinstance(elem)) { handle_sequence([](nb::handle v) { return nb::cast(v); }); } else if (nb::isinstance(elem)) { From ed6afa5a987cedf1a51a0476980fe8610b2c4c89 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Fri, 28 Aug 2026 21:07:35 +0200 Subject: [PATCH 06/14] Add unit-test that asserts Metadata follows the mapping protocol --- tests/test_bindings.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_bindings.py b/tests/test_bindings.py index ba314fb..72ea974 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -137,6 +137,22 @@ def test_field_generation(structured_in_and_out_fields): assert np.allclose(in_view + 1, out_view) +def test_metadata_mapping_protocol(structured_in_and_out_fields): + field, _ = structured_in_and_out_fields + metadata = field.metadata + metadata["source"] = "test" + metadata["level"] = 1 + + values = dict(metadata) + assert values["source"] == "test" + assert values["level"] == 1 + assert list(values) == metadata.keys() + assert list(metadata) == metadata.keys() + assert len(metadata) == len(values) + assert "source" in metadata + assert metadata["source"] == "test" + + def test_field_array_accepts_matching_dtype_and_false_copy(structured_in_and_out_fields): in_f, _ = structured_in_and_out_fields From 2fd129f6c02bef924fd64cdc1c5454e5f22e8ed5 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 10:49:52 +0200 Subject: [PATCH 07/14] More defensive path handling --- src/atlas4py/_atlas4py_Config.cpp | 3 ++- tests/test_bindings.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 4e14b3e..69fad32 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -301,7 +301,8 @@ void atlas4py::bind_Config( nb::module_& m ) { throw std::runtime_error("Only 'yaml' or 'json' format is supported"); } } - return atlas::util::Config( eckit::PathName{nb::cast( nb::str(path) )} ); + auto fspath = nb::module_::import_("os").attr("fspath")(path); + return atlas::util::Config( eckit::PathName{ nb::cast( nb::str(fspath) ) } ); }, "path"_a, "format"_a = "" ) .def( "__repr__", []( atlas::util::Config const& config ) { return "_atlas4py.Config("_s + nb::str( make_object( config ) ) + ")"_s; diff --git a/tests/test_bindings.py b/tests/test_bindings.py index 72ea974..d05fd5c 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -331,3 +331,15 @@ def test_config_from_file(tmp_path): assert outer["inner_option1"] == 3.14 assert outer["inner_option2"] is True assert outer["inner_option3"] == "added_value" + + +def test_config_from_file_rejects_non_pathlike_object(tmp_path): + test_config_yaml = tmp_path / "test_config.yaml" + test_config_yaml.write_text("option: value") + + class StringConvertiblePath: + def __str__(self): + return str(test_config_yaml) + + with pytest.raises(TypeError, match="expected str, bytes or os.PathLike object"): + atlas4py.Config.from_file(StringConvertiblePath()) From 59cecf5c3188f4956170fa298ad94c6feb0eabe0 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 10:55:14 +0200 Subject: [PATCH 08/14] Change error types to the mapping protocol expectations --- src/atlas4py/_atlas4py_Config.cpp | 8 ++++---- tests/test_bindings.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 69fad32..0eb6c20 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -140,7 +140,7 @@ nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ) return _toPyObject( values ); } else { - throw std::out_of_range( "type of value unsupported for key " + key ); + throw nb::type_error( ( "type of value unsupported for key " + key ).c_str() ); } } @@ -207,7 +207,7 @@ void config_set( eckit::LocalConfiguration& config, const std::string& key, nb:: } config.set(key, vec); } else { - throw std::out_of_range("Unsupported sequence element type for key '" + key + "': got type '" + py_type_str(elem) + "'"); + throw nb::type_error(("Unsupported sequence element type for key '" + key + "': got type '" + py_type_str(elem) + "'").c_str()); } } else if (nb::isinstance(value)) { eckit::LocalConfiguration subconfig; @@ -217,7 +217,7 @@ void config_set( eckit::LocalConfiguration& config, const std::string& key, nb:: } config.set(key, subconfig); } else { - throw std::out_of_range("type of value unsupported for key '" + key + "': got type '" + py_type_str(value) + "'"); + throw nb::type_error(("type of value unsupported for key '" + key + "': got type '" + py_type_str(value) + "'").c_str()); } } @@ -257,7 +257,7 @@ void atlas4py::bind_Config( nb::module_& m ) { .def( "__getitem__", []( eckit::Configuration const& config, std::string const& key ) -> nb::object { if ( !config.has( key ) ) - throw std::out_of_range( "key <" + key + "> could not be found" ); + throw nb::key_error( ( "key <" + key + "> could not be found" ).c_str() ); return _toPyObject( config, key ); } ) .def( "__iter__", diff --git a/tests/test_bindings.py b/tests/test_bindings.py index d05fd5c..ee66bf5 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -233,6 +233,17 @@ def test_config_mapping_protocol(): assert len(config) == 3 +def test_config_mapping_errors(): + config = atlas4py.Config() + + with pytest.raises(KeyError, match="missing"): + config["missing"] + + for value in (object(), [object()]): + with pytest.raises(TypeError): + config["unsupported"] = value + + def test_config_from_kwargs(): config = atlas4py.Config.from_kwargs(option1="value1", option2=42) config["option3"] = 3.14 From 6ed96dac19b15e8ce746e4de4e622c02e9d9b7b4 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 11:02:59 +0200 Subject: [PATCH 09/14] Fix handling of None value --- src/atlas4py/_atlas4py_Config.cpp | 5 ++++- tests/test_bindings.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 0eb6c20..ab0068b 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -97,7 +97,10 @@ nb::object _toPyObject( std::vector const& v ) { nb::object _toPyObject( eckit::Configuration const& v, std::string const& key ) { - if ( v.isSubConfiguration ( key ) ) { + if ( v.isNull( key ) ) { + return nb::none(); + } + else if ( v.isSubConfiguration ( key ) ) { return _toPyObject( v.getSubConfiguration( key ) ); } else if (v.isBoolean( key )) { diff --git a/tests/test_bindings.py b/tests/test_bindings.py index ee66bf5..20e412a 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -239,7 +239,7 @@ def test_config_mapping_errors(): with pytest.raises(KeyError, match="missing"): config["missing"] - for value in (object(), [object()]): + for value in (None, object(), [object()]): with pytest.raises(TypeError): config["unsupported"] = value @@ -274,6 +274,15 @@ def test_config_from_yaml(): assert outer["inner_option3"] == "added_value" +def test_config_null_from_yaml(): + config = atlas4py.Config.from_yaml("a: null") + + assert "a" in config + assert config["a"] is None + assert dict(config) == {"a": None} + assert repr(config) == "_atlas4py.Config({'a': None})" + + def test_config_boolean_list_roundtrip_from_yaml(): yaml_string = """ flags: From 48479a9358e80188da54d970cbf04400abb264bd Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 11:09:43 +0200 Subject: [PATCH 10/14] Remove make_Config and inline it at callsite --- src/atlas4py/_atlas4py_Config.cpp | 19 +++++++------------ src/atlas4py/_atlas4py_Config.hpp | 3 --- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index ab0068b..8027fe2 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -237,17 +237,6 @@ nb::object atlas4py::make_object( eckit::Configuration const& v ) { return _toPyObject( v ); } - -atlas::util::Config atlas4py::make_Config( nb::kwargs const& kwargs ) { - atlas::util::Config config; - for( const auto& pair : kwargs ) { - const auto key = nb::cast(pair.first); - const auto& value = pair.second; - config_set(config, key, value); - } - return config; -} - void atlas4py::bind_Config( nb::module_& m ) { using namespace nanobind::literals; @@ -288,7 +277,13 @@ void atlas4py::bind_Config( nb::module_& m ) { nb::class_( m, "Config" ) .def( nb::init() ) .def_static( "from_kwargs", []( nb::kwargs kwargs ) { - return make_Config(kwargs); + atlas::util::Config config; + for( const auto& pair : kwargs ) { + const auto key = nb::cast(pair.first); + const auto& value = pair.second; + config_set(config, key, value); + } + return config; } ) .def_static( "from_yaml", []( std::string const& yaml ) { return atlas::util::Config( eckit::YAMLConfiguration(yaml) ); diff --git a/src/atlas4py/_atlas4py_Config.hpp b/src/atlas4py/_atlas4py_Config.hpp index 82f2d66..f14762a 100644 --- a/src/atlas4py/_atlas4py_Config.hpp +++ b/src/atlas4py/_atlas4py_Config.hpp @@ -15,7 +15,4 @@ namespace atlas4py { // Convert eckit::Configuration to a corresponding Python object nb::object make_object( eckit::Configuration const& v ); - // Create an atlas::util::Config from Python keyword arguments - atlas::util::Config make_Config( nb::kwargs const& kwargs ); - } // namespace atlas4py From 4b7dad2e3255f125808afbdc20126ffb659f585b Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 11:33:50 +0200 Subject: [PATCH 11/14] Be more explicit that both yaml and json are parsed by the same parser --- src/atlas4py/_atlas4py_Config.cpp | 6 ++++-- tests/test_bindings.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 8027fe2..f85968e 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -296,12 +296,14 @@ void atlas4py::bind_Config( nb::module_& m ) { if ( !format.empty() ) { auto format_lowercase = to_lowercase(format); if (format_lowercase != "yaml" && format_lowercase != "json") { - throw std::runtime_error("Only 'yaml' or 'json' format is supported"); + throw nb::value_error("Only 'yaml' or 'json' format is supported"); } } auto fspath = nb::module_::import_("os").attr("fspath")(path); return atlas::util::Config( eckit::PathName{ nb::cast( nb::str(fspath) ) } ); - }, "path"_a, "format"_a = "" ) + }, "path"_a, "format"_a = "", + "Load a configuration file using eckit's YAML parser. Both YAML and JSON are supported; " + "format is an allowed-value guard and does not select a parser or validate the file contents." ) .def( "__repr__", []( atlas::util::Config const& config ) { return "_atlas4py.Config("_s + nb::str( make_object( config ) ) + ")"_s; } ); diff --git a/tests/test_bindings.py b/tests/test_bindings.py index 20e412a..a809be1 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -363,3 +363,14 @@ def __str__(self): with pytest.raises(TypeError, match="expected str, bytes or os.PathLike object"): atlas4py.Config.from_file(StringConvertiblePath()) + + +def test_config_from_file_format_is_guard(tmp_path): + test_config_yaml = tmp_path / "test_config.yaml" + test_config_yaml.write_text("option: value") + + assert atlas4py.Config.from_file(test_config_yaml, format="json")["option"] == "value" + with pytest.raises(ValueError, match="Only 'yaml' or 'json'"): + atlas4py.Config.from_file(test_config_yaml, format="toml") + + assert "does not select a parser or validate the file contents" in atlas4py.Config.from_file.__doc__ From e8a9101f02daee47458348e8db396716d8f03a08 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Sat, 29 Aug 2026 12:16:31 +0200 Subject: [PATCH 12/14] Replace atlas4py.Config.from_kwargs with constructor --- src/atlas4py/_atlas4py_Config.cpp | 7 +++---- tests/test_bindings.py | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index f85968e..0de7ff6 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -276,14 +276,13 @@ void atlas4py::bind_Config( nb::module_& m ) { nb::class_( m, "Config" ) .def( nb::init() ) - .def_static( "from_kwargs", []( nb::kwargs kwargs ) { - atlas::util::Config config; + .def( "__init__", []( atlas::util::Config* config, nb::kwargs kwargs ) { + new ( config ) atlas::util::Config(); for( const auto& pair : kwargs ) { const auto key = nb::cast(pair.first); const auto& value = pair.second; - config_set(config, key, value); + config_set(*config, key, value); } - return config; } ) .def_static( "from_yaml", []( std::string const& yaml ) { return atlas::util::Config( eckit::YAMLConfiguration(yaml) ); diff --git a/tests/test_bindings.py b/tests/test_bindings.py index a809be1..9973cb6 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -211,7 +211,7 @@ def test_config_set_python_types(): assert [dict(d) for d in c["list_of_dicts"]] == [{"a": 1}, {"b": 2}] def test_config_contains(): - config = atlas4py.Config.from_kwargs(option1="value1", option2=42) + config = atlas4py.Config(option1="value1", option2=42) assert "option1" in config assert "option2" in config assert "option3" not in config @@ -223,7 +223,7 @@ def test_config_contains(): def test_config_mapping_protocol(): - config = atlas4py.Config.from_kwargs(option1="value1", option2=42, option3=3.14) + config = atlas4py.Config(option1="value1", option2=42, option3=3.14) # dict() uses keys() + __getitem__ d = dict(config) assert d == {"option1": "value1", "option2": 42, "option3": 3.14} @@ -244,8 +244,8 @@ def test_config_mapping_errors(): config["unsupported"] = value -def test_config_from_kwargs(): - config = atlas4py.Config.from_kwargs(option1="value1", option2=42) +def test_config_constructor_with_kwargs(): + config = atlas4py.Config(option1="value1", option2=42) config["option3"] = 3.14 assert config["option1"] == "value1" assert config["option2"] == 42 From e88c54dadee04d20f9dd6ad839e04b890e883b90 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Tue, 1 Sep 2026 16:45:51 +0200 Subject: [PATCH 13/14] Make eckit.Configuration and eckit.LocalConfiguration private --- src/atlas4py/_atlas4py_Config.cpp | 8 ++++---- tests/test_bindings.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 0de7ff6..48fb8f9 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -240,7 +240,7 @@ nb::object atlas4py::make_object( eckit::Configuration const& v ) { void atlas4py::bind_Config( nb::module_& m ) { using namespace nanobind::literals; - nb::class_( m, "eckit.Configuration" ) + nb::class_( m, "_eckit_Configuration" ) .def( "keys", &eckit::Configuration::keys ) .def( "__contains__", []( eckit::Configuration const& config, std::string const& key ) { @@ -261,17 +261,17 @@ void atlas4py::bind_Config( nb::module_& m ) { return config.keys().size(); } ) .def( "__repr__", []( eckit::Configuration const& config ) { - return "_atlas4py.eckit.Configuration("_s + nb::str( make_object( config ) ) + ")"_s; + return "atlas4py._atlas4py._eckit_Configuration("_s + nb::str( make_object( config ) ) + ")"_s; } ); - nb::class_( m, "eckit.LocalConfiguration" ) + nb::class_( m, "_eckit_LocalConfiguration" ) .def( nb::init() ) .def( "__setitem__", []( eckit::LocalConfiguration& config, std::string const& key, nb::object value ) { config_set(config,key,value); } ) .def( "__repr__", []( eckit::LocalConfiguration const& config ) { - return "_atlas4py.eckit.LocalConfiguration("_s + nb::str( make_object( config ) ) + ")"_s; + return "atlas4py._atlas4py._eckit_LocalConfiguration("_s + nb::str( make_object( config ) ) + ")"_s; } ); nb::class_( m, "Config" ) diff --git a/tests/test_bindings.py b/tests/test_bindings.py index 9973cb6..8916333 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -233,6 +233,19 @@ def test_config_mapping_protocol(): assert len(config) == 3 +def test_config_eckit_base_classes_are_private(): + extension = atlas4py._atlas4py + + assert not hasattr(atlas4py, "_eckit_Configuration") + assert not hasattr(atlas4py, "_eckit_LocalConfiguration") + assert extension._eckit_Configuration.__module__ == "atlas4py._atlas4py" + assert extension._eckit_LocalConfiguration.__module__ == "atlas4py._atlas4py" + assert atlas4py.Config.__mro__[1:3] == ( + extension._eckit_LocalConfiguration, + extension._eckit_Configuration, + ) + + def test_config_mapping_errors(): config = atlas4py.Config() From a7a2d6b8905a51609c96f6c796f244b60b628d10 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Tue, 1 Sep 2026 20:48:43 +0200 Subject: [PATCH 14/14] Address #39 partially, only for the Config related classes --- src/atlas4py/_atlas4py.cpp | 2 +- src/atlas4py/_atlas4py_Config.cpp | 35 ++++++++++++++++++------------- tests/test_bindings.py | 27 +++++++++++++++++++++++- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/atlas4py/_atlas4py.cpp b/src/atlas4py/_atlas4py.cpp index aa9adad..8f5670b 100644 --- a/src/atlas4py/_atlas4py.cpp +++ b/src/atlas4py/_atlas4py.cpp @@ -474,7 +474,7 @@ NB_MODULE( _atlas4py, m ) { nb::class_( m, "Metadata" ) .def( "__repr__", []( util::Metadata const& metadata ) { - return "_atlas4py.Metadata("_s + nb::str( atlas4py::make_object( metadata ) ) + ")"_s; + return "atlas4py.Metadata("_s + nb::str( atlas4py::make_object( metadata ) ) + ")"_s; } ); nb::class_ topology( m, "Topology" ); diff --git a/src/atlas4py/_atlas4py_Config.cpp b/src/atlas4py/_atlas4py_Config.cpp index 48fb8f9..8ef7bb3 100644 --- a/src/atlas4py/_atlas4py_Config.cpp +++ b/src/atlas4py/_atlas4py_Config.cpp @@ -155,6 +155,7 @@ nb::object _toPyObject( eckit::Configuration const& v ) { return ret; } +void config_update( eckit::LocalConfiguration& config, nb::handle mapping ); void config_set( eckit::LocalConfiguration& config, const std::string& key, nb::handle value ) { auto py_type_str = [](nb::handle h) -> std::string { @@ -202,10 +203,7 @@ void config_set( eckit::LocalConfiguration& config, const std::string& key, nb:: std::vector vec; for (size_t i = 0; i < n; ++i) { eckit::LocalConfiguration subconfig; - nb::object mapping = nb::cast(seq[i]); - for (nb::handle item : mapping.attr("keys")()) { - config_set(subconfig, nb::cast(item), mapping[item]); - } + config_update(subconfig, seq[i]); vec.push_back(subconfig); } config.set(key, vec); @@ -214,16 +212,25 @@ void config_set( eckit::LocalConfiguration& config, const std::string& key, nb:: } } else if (nb::isinstance(value)) { eckit::LocalConfiguration subconfig; - nb::object mapping = nb::cast(value); - for (nb::handle item : mapping.attr("keys")()) { - config_set(subconfig, nb::cast(item), mapping[item]); - } + config_update(subconfig, value); config.set(key, subconfig); } else { throw nb::type_error(("type of value unsupported for key '" + key + "': got type '" + py_type_str(value) + "'").c_str()); } } +void config_update( eckit::LocalConfiguration& config, nb::handle mapping ) { + if ( !nb::isinstance( mapping ) ) { + throw nb::type_error( "expected a mapping" ); + } + for ( nb::handle key : mapping.attr( "keys" )() ) { + if ( !nb::isinstance( key ) ) { + throw nb::type_error( "configuration keys must be strings" ); + } + config_set( config, nb::cast( key ), mapping[key] ); + } +} + std::string to_lowercase(const std::string& str) { std::string lowercase_str = str; std::transform(lowercase_str.begin(), lowercase_str.end(), lowercase_str.begin(), @@ -276,13 +283,13 @@ void atlas4py::bind_Config( nb::module_& m ) { nb::class_( m, "Config" ) .def( nb::init() ) + .def( "__init__", []( atlas::util::Config* config, nb::object mapping ) { + new ( config ) atlas::util::Config(); + config_update( *config, mapping ); + }, "mapping"_a ) .def( "__init__", []( atlas::util::Config* config, nb::kwargs kwargs ) { new ( config ) atlas::util::Config(); - for( const auto& pair : kwargs ) { - const auto key = nb::cast(pair.first); - const auto& value = pair.second; - config_set(*config, key, value); - } + config_update( *config, kwargs ); } ) .def_static( "from_yaml", []( std::string const& yaml ) { return atlas::util::Config( eckit::YAMLConfiguration(yaml) ); @@ -304,6 +311,6 @@ void atlas4py::bind_Config( nb::module_& m ) { "Load a configuration file using eckit's YAML parser. Both YAML and JSON are supported; " "format is an allowed-value guard and does not select a parser or validate the file contents." ) .def( "__repr__", []( atlas::util::Config const& config ) { - return "_atlas4py.Config("_s + nb::str( make_object( config ) ) + ")"_s; + return "atlas4py.Config("_s + nb::str( make_object( config ) ) + ")"_s; } ); } diff --git a/tests/test_bindings.py b/tests/test_bindings.py index 8916333..1d818bb 100644 --- a/tests/test_bindings.py +++ b/tests/test_bindings.py @@ -233,6 +233,15 @@ def test_config_mapping_protocol(): assert len(config) == 3 +def test_config_repr_roundtrip(): + config = atlas4py.Config(option1="value1", option2=42, option3=3.14) + + reconstructed = eval(repr(config)) + + assert isinstance(reconstructed, atlas4py.Config) + assert dict(reconstructed) == dict(config) + + def test_config_eckit_base_classes_are_private(): extension = atlas4py._atlas4py @@ -265,6 +274,22 @@ def test_config_constructor_with_kwargs(): assert config["option3"] == 3.14 assert config.keys() == ["option1", "option2", "option3"] + +def test_config_constructor_with_mapping(): + from collections import UserDict + + values = {"option1": "value1", "option2": 42} + + assert dict(atlas4py.Config(values)) == values + assert dict(atlas4py.Config(UserDict(values))) == values + + with pytest.raises(TypeError, match="expected a mapping"): + atlas4py.Config(object()) + + for values in ({1: "value"}, {"nested": {1: "value"}}, {"nested": [{1: "value"}]}): + with pytest.raises(TypeError, match="configuration keys must be strings"): + atlas4py.Config(values) + def test_config_from_yaml(): yaml_string = """ option1: value1 @@ -293,7 +318,7 @@ def test_config_null_from_yaml(): assert "a" in config assert config["a"] is None assert dict(config) == {"a": None} - assert repr(config) == "_atlas4py.Config({'a': None})" + assert repr(config) == "atlas4py.Config({'a': None})" def test_config_boolean_list_roundtrip_from_yaml():