From cf192b80cf441c0b8a2451a2b16c85db1e8e21ca Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:07:10 +0200 Subject: [PATCH] refactor(config): key microgrids by int Normalize microgrid map keys to plain integers, matching metadata and relation IDs. Keep string keys only at TOML and serialization boundaries. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- RELEASE_NOTES.md | 8 +++++++ src/frequenz/gridpool/cli/__main__.py | 4 +--- src/frequenz/gridpool/cli/_dump_config.py | 8 +++---- src/frequenz/gridpool/cli/_patch_config.py | 11 +++++---- src/frequenz/gridpool/config/_assets.py | 6 ++--- src/frequenz/gridpool/config/_load.py | 19 ++++++++------- src/frequenz/gridpool/config/_microgrid.py | 4 ++-- tests/test_config.py | 28 +++++++++++----------- tests/test_dump_config.py | 6 ++--- tests/test_load.py | 14 ++++++----- tests/test_patch_config.py | 14 +++++------ tests/test_topology.py | 2 +- 12 files changed, 66 insertions(+), 58 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 2b82407..8fac4a4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -45,6 +45,14 @@ - The implementation modules `config.load` and `config.microgrid` are now private. Import their public names from `frequenz.gridpool.config` instead. +- Microgrids are now keyed by `int` microgrid ID, not `str`. This covers + `AssetsConfig.microgrids`, including documents returned by `load_configs`. + Index the mapping by integer ID: + + ```python + configs[1] # was configs["1"] + ``` + ## New Features - `AssetsConfig` gives the `assets` namespace a type, so the entities still to diff --git a/src/frequenz/gridpool/cli/__main__.py b/src/frequenz/gridpool/cli/__main__.py index a240bed..425d3a3 100644 --- a/src/frequenz/gridpool/cli/__main__.py +++ b/src/frequenz/gridpool/cli/__main__.py @@ -203,9 +203,7 @@ async def generate_config( ids = list(dict.fromkeys(microgrid_ids)) or None if inplace and ids is None: assert default_file is not None - ids = sorted( - int(mid) for mid in AssetsConfig.load_from_files(default_file).microgrids - ) + ids = sorted(AssetsConfig.load_from_files(default_file).microgrids) async with AssetsApiClient(url, auth_key=key, sign_secret=secret) as client: if inplace: diff --git a/src/frequenz/gridpool/cli/_dump_config.py b/src/frequenz/gridpool/cli/_dump_config.py index 5c9b7c4..4af0f21 100644 --- a/src/frequenz/gridpool/cli/_dump_config.py +++ b/src/frequenz/gridpool/cli/_dump_config.py @@ -71,11 +71,11 @@ def _iter_leaves( return leaves -def dump_map(configs: dict[str, MicrogridConfig]) -> str: +def dump_map(configs: dict[int, MicrogridConfig]) -> str: """Serialize a mapping of microgrid configs to dotted-key TOML. Args: - configs: Mapping from microgrid ID (as string) to `MicrogridConfig`. + configs: Mapping from microgrid ID to `MicrogridConfig`. Returns: The TOML representation as a string, with one blank line between @@ -83,10 +83,10 @@ def dump_map(configs: dict[str, MicrogridConfig]) -> str: """ schema = MicrogridConfig.Schema() doc = tomlkit.document() - for mid in sorted(configs, key=int): + for mid in sorted(configs): dumped = schema.dump(configs[mid]) assert isinstance(dumped, dict) - leaves = _iter_leaves([mid], dumped) + leaves = _iter_leaves([str(mid)], dumped) if not leaves: continue if doc.body: diff --git a/src/frequenz/gridpool/cli/_patch_config.py b/src/frequenz/gridpool/cli/_patch_config.py index 1a4d3e7..ed28da8 100644 --- a/src/frequenz/gridpool/cli/_patch_config.py +++ b/src/frequenz/gridpool/cli/_patch_config.py @@ -20,12 +20,12 @@ from ._dump_config import _format_value, _iter_leaves -def patch_file(path: Path, configs: dict[str, MicrogridConfig]) -> str: +def patch_file(path: Path, configs: dict[int, MicrogridConfig]) -> str: """Patch the TOML file at `path` with any leaves missing from `configs`. Args: path: Path to the existing TOML file to patch. - configs: Mapping from microgrid ID (as string) to `MicrogridConfig`. + configs: Mapping from microgrid ID to `MicrogridConfig`. Returns: The patched TOML text; the caller is responsible for writing it back. @@ -33,12 +33,12 @@ def patch_file(path: Path, configs: dict[str, MicrogridConfig]) -> str: return patch_text(path.read_text(), configs) -def patch_text(original: str, configs: dict[str, MicrogridConfig]) -> str: +def patch_text(original: str, configs: dict[int, MicrogridConfig]) -> str: """Patch dotted-key TOML text with any leaves missing from `configs`. Args: original: The existing TOML text to patch. - configs: Mapping from microgrid ID (as string) to `MicrogridConfig`. + configs: Mapping from microgrid ID to `MicrogridConfig`. Returns: The patched TOML text. @@ -51,7 +51,8 @@ def patch_text(original: str, configs: dict[str, MicrogridConfig]) -> str: # into the rendered text instead. orphans: dict[str, list[tuple[list[str], Any]]] = {} - for mid, cfg in configs.items(): + for microgrid_id, cfg in configs.items(): + mid = str(microgrid_id) dumped = schema.dump(cfg) assert isinstance(dumped, dict) leaves = _iter_leaves([], dumped) diff --git a/src/frequenz/gridpool/config/_assets.py b/src/frequenz/gridpool/config/_assets.py index 8a8f629..ec859f2 100644 --- a/src/frequenz/gridpool/config/_assets.py +++ b/src/frequenz/gridpool/config/_assets.py @@ -82,7 +82,7 @@ def _merge_file_tables( class AssetsConfig: """Entities described by a config document, keyed by their ID.""" - microgrids: dict[str, MicrogridConfig] = field(default_factory=dict) + microgrids: dict[int, MicrogridConfig] = field(default_factory=dict) """Microgrids, keyed by microgrid ID.""" market_locations: dict[str, MarketLocationConfig] = field(default_factory=dict) @@ -115,9 +115,7 @@ def __post_init__(self) -> None: ValueError: If a key is not the ID of the entry it holds. """ for mid, cfg in self.microgrids.items(): - if not mid.isdigit(): - raise ValueError(f"Microgrid ID key must be numeric, got {mid}") - if int(cfg.meta.microgrid_id) != int(mid): + if int(cfg.meta.microgrid_id) != mid: raise ValueError( f"Microgrid ID mismatch: key {mid} != {cfg.meta.microgrid_id}" ) diff --git a/src/frequenz/gridpool/config/_load.py b/src/frequenz/gridpool/config/_load.py index 4ccac04..25b6952 100644 --- a/src/frequenz/gridpool/config/_load.py +++ b/src/frequenz/gridpool/config/_load.py @@ -126,7 +126,9 @@ async def load_configs( ) schema = MicrogridConfig.Schema() api_table: dict[str, Any] = { - "microgrids": {mid: schema.dump(cfg) for mid, cfg in assets_configs.items()} + "microgrids": { + str(mid): schema.dump(cfg) for mid, cfg in assets_configs.items() + } } merged = _deep_merge(merged, api_table) @@ -142,7 +144,7 @@ async def _load_microgrids_from_api( assets_client: AssetsApiClient, microgrid_ids: list[int], component_graph_config: ComponentGraphConfig | None = None, -) -> dict[str, "MicrogridConfig"]: +) -> dict[int, "MicrogridConfig"]: """Load microgrid configs from the Assets API. For each microgrid, fetches its location metadata (latitude, longitude) and @@ -165,14 +167,13 @@ async def _load_microgrids_from_api( `ComponentGraphConfig`. Defaults to that class's own defaults. Returns: - dict[str, MicrogridConfig]: - Mapping from microgrid ID (as string) to the loaded - `MicrogridConfig` instance. Microgrids whose metadata could not be - loaded are omitted, so the returned mapping may cover fewer - microgrids than were requested. + dict[int, MicrogridConfig]: + Mapping from microgrid ID to the loaded `MicrogridConfig` instance. + Microgrids whose metadata could not be loaded are omitted, so the + returned mapping may cover fewer microgrids than were requested. """ generator = ComponentGraphGenerator(assets_client, config=component_graph_config) - configs: dict[str, MicrogridConfig] = {} + configs: dict[int, MicrogridConfig] = {} for microgrid_id in microgrid_ids: try: cfg = await _build_config_from_metadata(assets_client, microgrid_id) @@ -195,7 +196,7 @@ async def _load_microgrids_from_api( exc, ) - configs[str(microgrid_id)] = cfg + configs[microgrid_id] = cfg return configs diff --git a/src/frequenz/gridpool/config/_microgrid.py b/src/frequenz/gridpool/config/_microgrid.py index 5ad520c..8162094 100644 --- a/src/frequenz/gridpool/config/_microgrid.py +++ b/src/frequenz/gridpool/config/_microgrid.py @@ -291,7 +291,7 @@ def formula(self, component_type: str, metric: str) -> str: Schema: ClassVar[Type[Schema]] = Schema @classmethod - def _load_table_entries(cls, data: dict[str, Any]) -> dict[str, Self]: + def _load_table_entries(cls, data: dict[str, Any]) -> dict[int, Self]: """Load microgrid configurations from table entries. Args: @@ -327,6 +327,6 @@ def _load_table_entries(cls, data: dict[str, Any]) -> dict[str, Self]: f"Table reader: Microgrid ID mismatch: key {mid} != {mgrid.meta.microgrid_id}" ) - mgrids[mid] = mgrid + mgrids[int(mid)] = mgrid return mgrids diff --git a/tests/test_config.py b/tests/test_config.py index a31817a..aa13678 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -48,7 +48,7 @@ def valid_microgrid_config() -> MicrogridConfig: """Fixture to provide a valid MicrogridConfig instance.""" # pylint: disable=protected-access - return MicrogridConfig._load_table_entries(VALID_CONFIG)["1"] + return MicrogridConfig._load_table_entries(VALID_CONFIG)[1] def test_is_valid_type() -> None: @@ -136,17 +136,17 @@ def test_load_configs(mocker: MockerFixture) -> None: mocker.patch("pathlib.Path.is_file", mocker.Mock(return_value=True)) configs = AssetsConfig.load_from_files(Path("mock_path.toml")).microgrids - assert "1" in configs - assert configs["1"].meta is not None - assert configs["1"].meta.name == "Test Grid" + assert 1 in configs + assert configs[1].meta is not None + assert configs[1].meta.name == "Test Grid" - pv_config = configs["1"].pv + pv_config = configs[1].pv assert pv_config is not None pv_system = pv_config.get("PV1") assert pv_system is not None assert pv_system.peak_power == 5000 - battery_config = configs["1"].battery + battery_config = configs[1].battery assert battery_config is not None battery_system = battery_config.get("BAT1") assert battery_system is not None @@ -194,8 +194,8 @@ def test_load_prefixed(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None _write(tmp_path, "prefixed.toml", _PREFIXED_TOML) ).microgrids - assert configs["1"].meta.name == "Test Grid" - assert configs["1"].component_type_ids("pv") == [101, 102] + assert configs[1].meta.name == "Test Grid" + assert configs[1].component_type_ids("pv") == [101, 102] assert "deprecated" not in caplog.text @@ -206,7 +206,7 @@ def test_load_legacy_warns(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> with caplog.at_level(logging.WARNING): configs = AssetsConfig.load_from_files(path).microgrids - assert configs["1"].meta.name == "Test Grid" + assert configs[1].meta.name == "Test Grid" assert "deprecated" in caplog.text assert str(path) in caplog.text @@ -241,8 +241,8 @@ async def test_merge_prefixed_base_with_legacy_override(tmp_path: Path) -> None: await load_configs(default_files=base, override_files=override) ).microgrids - assert configs["1"].meta.name == "Renamed" - assert configs["1"].component_type_ids("pv") == [101, 102] + assert configs[1].meta.name == "Renamed" + assert configs[1].component_type_ids("pv") == [101, 102] def test_load_from_files_layers_fields(tmp_path: Path) -> None: @@ -257,8 +257,8 @@ def test_load_from_files_layers_fields(tmp_path: Path) -> None: configs = AssetsConfig.load_from_files([base, override]).microgrids - assert configs["1"].meta.name == "Renamed" - assert configs["1"].component_type_ids("pv") == [101, 102] + assert configs[1].meta.name == "Renamed" + assert configs[1].component_type_ids("pv") == [101, 102] def test_assets_config_rejects_mismatched_id() -> None: @@ -292,5 +292,5 @@ def test_assets_config_warns_on_unknown_entities( with caplog.at_level(logging.WARNING): config = AssetsConfig.load_from_files(path) - assert sorted(config.microgrids) == ["1"] + assert sorted(config.microgrids) == [1] assert "gridpool" in caplog.text diff --git a/tests/test_dump_config.py b/tests/test_dump_config.py index daa3c5b..d10cd18 100644 --- a/tests/test_dump_config.py +++ b/tests/test_dump_config.py @@ -17,7 +17,7 @@ def test_dump_map_round_trips() -> None: """A serialized config parses back to the same dotted-key structure.""" configs = { - "10": MicrogridConfig( + 10: MicrogridConfig( meta=Metadata(microgrid_id=10, name="Demo", latitude=52.5), ctype={ "pv": ComponentTypeConfig(meter=[2], formula={"AC_POWER_ACTIVE": "#2"}), @@ -36,7 +36,7 @@ def test_dump_map_round_trips() -> None: def test_dump_map_omits_empty_and_none() -> None: """Empty and None fields are dropped from the output.""" - configs = {"7": MicrogridConfig(meta=Metadata(microgrid_id=7))} + configs = {7: MicrogridConfig(meta=Metadata(microgrid_id=7))} text = dump_map(configs) @@ -51,7 +51,7 @@ def test_dump_map_empty() -> None: def test_dump_map_renders_whole_floats_as_underscored_ints() -> None: """Whole-number float fields (e.g. peak/rated power) render as `1_736_680`, not `1736680.0`.""" configs = { - "10": MicrogridConfig( + 10: MicrogridConfig( meta=Metadata(microgrid_id=10, latitude=52.5), pv={"1": PVConfig(peak_power=1_736_680.0, rated_power=1_400_000.0)}, ) diff --git a/tests/test_load.py b/tests/test_load.py index f94db3c..bb2a384 100644 --- a/tests/test_load.py +++ b/tests/test_load.py @@ -69,7 +69,7 @@ async def test_load_microgrids_from_api_derives_formulas_and_ids() -> None: """A config loaded from the API gets both formulas and component IDs.""" configs = await _load_microgrids_from_api(_mock_client(), [10]) - cfg = configs["10"] + cfg = configs[10] assert cfg.ctype["pv"].formula == {"AC_POWER_ACTIVE": "COALESCE(#4, #2, 0.0)"} assert cfg.ctype["pv"].inverter == [4] assert cfg.ctype["pv"].meter == [2] @@ -89,7 +89,7 @@ async def test_load_microgrids_from_api_honours_the_component_graph_config() -> ) # Meter first, the opposite of the default order asserted above. - ctype = configs["10"].ctype + ctype = configs[10].ctype assert ctype["pv"].formula == {"AC_POWER_ACTIVE": "COALESCE(#2, #4, 0.0)"} @@ -105,7 +105,7 @@ async def test_load_configs_forwards_the_component_graph_config() -> None: ) ).microgrids - assert configs["10"].ctype["pv"].formula == { + assert configs[10].ctype["pv"].formula == { "AC_POWER_ACTIVE": "COALESCE(#2, #4, 0.0)" } @@ -128,7 +128,7 @@ async def test_load_microgrids_from_api_keeps_metadata_when_graph_fails() -> Non configs = await _load_microgrids_from_api(client, [10]) - cfg = configs["10"] + cfg = configs[10] assert cfg.meta.microgrid_id == 10 assert cfg.ctype == {} @@ -149,7 +149,7 @@ async def test_load_configs_validates_the_merged_whole(tmp_path: Path) -> None: document = await load_configs(default_files=default, override_files=override) - assert document.microgrids["1"].meta.name == "Override" + assert document.microgrids[1].meta.name == "Override" async def test_load_configs_returns_the_whole_document(tmp_path: Path) -> None: @@ -161,6 +161,7 @@ async def test_load_configs_returns_the_whole_document(tmp_path: Path) -> None: default = tmp_path / "default.toml" default.write_text( "assets.microgrids.10.meta.microgrid_id = 10\n" + 'assets.microgrids.10.meta.name = "File name"\n' 'assets.market_locations.51171875559.id = "51171875559"\n' "assets.relations.M10L51171875559.microgrid_id = 10\n" 'assets.relations.M10L51171875559.market_location_id = "51171875559"\n' @@ -169,7 +170,8 @@ async def test_load_configs_returns_the_whole_document(tmp_path: Path) -> None: document = await load_configs(default_files=default, assets_client=_mock_client()) # The API layer filled the microgrid's component config. - assert document.microgrids["10"].ctype + assert document.microgrids[10].ctype + assert document.microgrids[10].meta.name == "File name" # The file's topology survived the merge with the API layer. assert "M10L51171875559" in document.relations assert "51171875559" in document.market_locations diff --git a/tests/test_patch_config.py b/tests/test_patch_config.py index 476ec88..001e8f3 100644 --- a/tests/test_patch_config.py +++ b/tests/test_patch_config.py @@ -28,7 +28,7 @@ def test_patch_is_a_noop_when_nothing_changed() -> None: """Patching with values already on disk leaves the file byte-identical.""" configs = { - "40": MicrogridConfig( + 40: MicrogridConfig( meta=Metadata(microgrid_id=40, latitude=50.39567065), pv={"1": PVConfig(peak_power=616_140.0, rated_power=480_000.0)}, ) @@ -40,7 +40,7 @@ def test_patch_is_a_noop_when_nothing_changed() -> None: def test_patch_inserts_missing_leaf_next_to_existing_table() -> None: """A missing leaf under an existing table is inserted; everything else is untouched.""" configs = { - "40": MicrogridConfig(meta=Metadata(microgrid_id=40, altitude=45.5)), + 40: MicrogridConfig(meta=Metadata(microgrid_id=40, altitude=45.5)), } patched = patch_text(_ORIGINAL, configs) @@ -59,7 +59,7 @@ def test_patch_inserts_missing_leaf_next_to_existing_table() -> None: def test_patch_appends_new_microgrid_at_the_end() -> None: """A microgrid id absent from the file is appended, blank-line separated.""" configs = { - "9999": MicrogridConfig(meta=Metadata(microgrid_id=9999, name="Brand New")), + 9999: MicrogridConfig(meta=Metadata(microgrid_id=9999, name="Brand New")), } patched = patch_text(_ORIGINAL, configs) @@ -73,7 +73,7 @@ def test_patch_appends_new_microgrid_at_the_end() -> None: def test_patch_inserts_new_subtable_next_to_its_microgrid() -> None: """A brand-new sub-table for an existing id lands next to that id's other lines.""" configs = { - "40": MicrogridConfig( + 40: MicrogridConfig( meta=Metadata(microgrid_id=40), pv={"2": PVConfig(peak_power=50_000.0)}, ), @@ -88,11 +88,11 @@ def test_patch_inserts_new_subtables_for_multiple_microgrids() -> None: """Each microgrid's new sub-table lands next to its own lines, not all at the end.""" original = _ORIGINAL + '\n41.meta.name = "Other Grid"\n41.meta.microgrid_id = 41\n' configs = { - "40": MicrogridConfig( + 40: MicrogridConfig( meta=Metadata(microgrid_id=40), pv={"2": PVConfig(peak_power=50_000.0)}, ), - "41": MicrogridConfig( + 41: MicrogridConfig( meta=Metadata(microgrid_id=41), ctype={"grid": ComponentTypeConfig(meter=[1])}, ), @@ -113,7 +113,7 @@ def test_patch_inserts_new_subtables_for_multiple_microgrids() -> None: def test_patch_formats_new_numeric_leaves() -> None: """Newly inserted numeric leaves go through the same underscore formatting.""" configs = { - "5555": MicrogridConfig( + 5555: MicrogridConfig( meta=Metadata(microgrid_id=5555, enterprise_id=1_234_567) ), } diff --git a/tests/test_topology.py b/tests/test_topology.py index e17e427..c6e124a 100644 --- a/tests/test_topology.py +++ b/tests/test_topology.py @@ -346,7 +346,7 @@ def test_legacy_gridpool_id_must_match_relations() -> None: {"gridpool_id": 80, "microgrid_id": 241, "delivery_area": {"code": _AREA_A}} ), ) - assert config.microgrids["241"].meta.gid == 80 + assert config.microgrids[241].meta.gid == 80 _load( microgrids={"241": {"meta": {"microgrid_id": 241}}},