From 8d959ac5bc9d3a412a49ab8a596ded4477ecec2a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 6 Jul 2026 12:41:02 +0000 Subject: [PATCH 1/4] Make `Microgrid.name` non-optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the type of `Microgrid.name` from `str | None` to `str`. Represent the absence of a name as `""` (matching the protobuf default) rather than `None`, so consumers don't have to distinguish between "no name" and "empty name" — the two are semantically identical. `microgrid_from_proto` now assigns `message.name` directly since protobuf string fields default to `""` when unset. Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/_microgrid.py | 2 +- .../common/microgrid/proto/v1alpha8/_microgrid.py | 2 +- tests/microgrid/proto/v1alpha8/test_microgrid.py | 2 +- tests/microgrid/test_microgrid.py | 15 +++++++-------- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index af5060e..21d6641 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -37,7 +37,7 @@ class Microgrid: # pylint: disable=too-many-instance-attributes enterprise_id: EnterpriseId """The unique identifier linking this microgrid to its parent enterprise account.""" - name: str | None + name: str """The name of the microgrid.""" delivery_area: DeliveryArea | None diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index f9ff2db..37a703b 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -92,7 +92,7 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: return Microgrid( id=MicrogridId(message.id), enterprise_id=EnterpriseId(message.enterprise_id), - name=message.name or None, + name=message.name, delivery_area=delivery_area, location=location, create_time=datetime_from_proto(message.create_timestamp), diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index 93b83d7..e5b8017 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -213,7 +213,7 @@ def test_from_proto( if case.has_name: assert info.name == "Test Grid" else: - assert info.name is None + assert info.name == "" _assert_active(info, case.expected_active) diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index cd5ebff..a24b091 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -55,7 +55,7 @@ def test_creation_without_optionals() -> None: info = Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, @@ -65,7 +65,7 @@ def test_creation_without_optionals() -> None: assert info.id == MicrogridId(1234) assert info.enterprise_id == EnterpriseId(5678) - assert info.name is None + assert info.name == "" assert info.delivery_area is None assert info.location is None assert info.create_time == now @@ -85,7 +85,7 @@ def test_is_active(active: bool) -> None: info = Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, @@ -101,7 +101,7 @@ def test_is_active_unspecified() -> None: info = Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, @@ -134,11 +134,10 @@ def test_is_active_unrecognized() -> None: "name,expected_str", [ pytest.param("Test Grid", "MID1234:Test Grid", id="with-name"), - pytest.param(None, "MID1234", id="none-name"), pytest.param("", "MID1234", id="empty-name"), ], ) -def test_str(name: str | None, expected_str: str) -> None: +def test_str(name: str, expected_str: str) -> None: """Test string representation of Microgrid.""" now = datetime.now(timezone.utc) info = Microgrid( @@ -161,7 +160,7 @@ def test_direct_construction_raises() -> None: Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, @@ -175,7 +174,7 @@ def test_replace_preserves_construction() -> None: info = Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, From 1b2a2651fb6200a929a3132dd06e411c0a2621db Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 6 Jul 2026 12:41:42 +0000 Subject: [PATCH 2/4] Stop reporting empty microgrid names as an issue Now that `Microgrid.name` is `str` and represents the absence of a name as `""`, having no name is not an issue worth flagging. Drop the `minor_issues` bookkeeping in `microgrid_from_proto` (it was only used for the empty-name case) and the accompanying DEBUG log. Signed-off-by: Leandro Lucarella --- .../common/microgrid/proto/v1alpha8/_microgrid.py | 12 ------------ tests/microgrid/proto/v1alpha8/test_microgrid.py | 1 - 2 files changed, 13 deletions(-) diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index 37a703b..cca25f1 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -50,7 +50,6 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: The corresponding [`Microgrid`][....Microgrid] object. """ major_issues: list[str] = [] - minor_issues: list[str] = [] delivery_area: DeliveryArea | None = None if message.HasField("delivery_area"): @@ -64,10 +63,6 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: else: major_issues.append("location is missing") - name = message.name or None - if name is None: - minor_issues.append("name is empty") - active = _microgrid_status_to_active(message.status) if message.status == microgrid_pb2.MICROGRID_STATUS_UNSPECIFIED: major_issues.append("status is unspecified") @@ -81,13 +76,6 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: message, ) - if minor_issues: - _logger.debug( - "Found minor issues in microgrid: %s | Protobuf message:\n%s", - ", ".join(minor_issues), - message, - ) - # The wrapper uses create_time, but the protobuf field remains create_timestamp. return Microgrid( id=MicrogridId(message.id), diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index e5b8017..9c60c1e 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -115,7 +115,6 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: has_name=False, status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, expected_active=True, - expected_log=("DEBUG", "Found minor issues in microgrid: name is empty"), ), _ProtoConversionTestCase( name="unspecified_status", From 509d1aac02ae91c33e218aa22284c46afa6791c0 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 6 Jul 2026 12:46:39 +0000 Subject: [PATCH 3/4] Make `ElectricalComponent.name` non-optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the type of `ElectricalComponent.name` from `str | None` to `str`. Represent the absence of a name as `""` (matching the protobuf default) rather than `None`, so consumers don't have to distinguish between "no name" and "empty name" — the two are semantically identical. The intermediate `_ElectricalComponentBaseData.name` and the proto conversion helpers are updated to match; tests that constructed concrete components without a name now pass `name=""` explicitly. Signed-off-by: Leandro Lucarella --- .../electrical_components/_electrical_component.py | 2 +- .../proto/v1alpha8/_electrical_component.py | 8 ++++---- .../proto/v1alpha8/conftest.py | 2 +- .../proto/v1alpha8/test_class.py | 1 + .../v1alpha8/test_electrical_component_base.py | 2 +- .../proto/v1alpha8/test_raw_storage.py | 3 +++ .../electrical_components/test_battery.py | 3 +++ .../test_electrical_component_base.py | 14 +++++++++++--- .../electrical_components/test_ev_charger.py | 3 +++ .../test_grid_connection_point.py | 1 + .../electrical_components/test_inverter.py | 3 +++ tests/microgrid/test_microgrid.py | 2 +- 12 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 62f4833..6d45858 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -25,7 +25,7 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes microgrid_id: MicrogridId """The ID of the microgrid this electrical component belongs to.""" - name: str | None = None + name: str """The name of this electrical component.""" model: str | None = None diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 1cbb0bd..e2c853d 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -878,8 +878,8 @@ class _ElectricalComponentBaseData(NamedTuple): microgrid_id: MicrogridId """The unique identifier of the parent microgrid.""" - name: str | None - """The optional human-readable name of the electrical component.""" + name: str + """The human-readable name of the electrical component.""" model: str | None """The optional model string of the electrical component.""" @@ -928,8 +928,8 @@ def _electrical_component_base_from_proto_with_issues( component_id = ElectricalComponentId(message.id) microgrid_id = MicrogridId(message.microgrid_id) - name = message.name or None - if name is None: + name = message.name + if not name: minor_issues.append("name is empty") model = message.model or None diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 93586f5..8182830 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -127,7 +127,7 @@ def base_data_as_proto( proto = electrical_components_pb2.ElectricalComponent( id=int(base_data.component_id), microgrid_id=int(base_data.microgrid_id), - name=base_data.name or "", + name=base_data.name, model=base_data.model or "", category=electrical_components_pb2.ElectricalComponentCategory.ValueType( base_data.category diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py index a66ba19..5815dd5 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py @@ -180,6 +180,7 @@ _BASE_KWARGS: dict[str, object] = { "id": ElectricalComponentId(1), "microgrid_id": MicrogridId(1), + "name": "", "_provides_telemetry": True, "_accepts_control": True, "_allow_construction": True, diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 35b82c8..48011b3 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -96,7 +96,7 @@ def test_missing_category_specific_info( major_issues: list[str] = [] minor_issues: list[str] = [] base_data = default_component_base_data._replace( - name=None, + name="", category=ElectricalComponentCategory.UNSPECIFIED, lifetime=Lifetime(), metric_config_bounds={}, diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py index db5d6ce..01627c1 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py @@ -61,6 +61,7 @@ def test_raw_type_preserved_for_unknown( component = UnrecognizedBattery( id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, + name="", type=999, _provides_telemetry=True, _accepts_control=True, @@ -92,6 +93,7 @@ def test_unrecognized_type_shows_in_repr( component = UnrecognizedBattery( id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, + name="", type=999, _provides_telemetry=True, _accepts_control=True, @@ -109,6 +111,7 @@ def _unrecognized_battery(battery_type: int) -> UnrecognizedBattery: return UnrecognizedBattery( id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, + name="", type=battery_type, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_battery.py b/tests/microgrid/electrical_components/test_battery.py index 4a8b535..4e47d85 100644 --- a/tests/microgrid/electrical_components/test_battery.py +++ b/tests/microgrid/electrical_components/test_battery.py @@ -95,6 +95,7 @@ def test_unspecified_battery_is_problematic( battery = UnspecifiedBattery( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -111,6 +112,7 @@ def test_unrecognized_battery_is_problematic( battery = UnrecognizedBattery( id=component_id, microgrid_id=microgrid_id, + name="", type=999, _provides_telemetry=True, _accepts_control=True, @@ -131,6 +133,7 @@ def test_recognized_battery_types_are_not_problematic( battery = cls( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 9079303..d18c68c 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -33,6 +33,7 @@ def test_base_creation_fails() -> None: _ = ElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), + name="", _provides_telemetry=True, _accepts_control=True, ) @@ -44,6 +45,7 @@ def test_direct_construction_without_flag_raises() -> None: _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), + name="", _provides_telemetry=True, _accepts_control=True, ) @@ -54,12 +56,13 @@ def test_creation_with_defaults() -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, ) - assert component.name is None + assert component.name == "" assert component.model is None assert component.operational_lifetime == Lifetime() assert component.metric_config_bounds == {} @@ -95,6 +98,7 @@ def test_accessors_return_values_when_set() -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), + name="", _provides_telemetry=True, _accepts_control=False, _allow_construction=True, @@ -109,6 +113,7 @@ def test_accessors_raise_when_unspecified() -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), + name="", _provides_telemetry=0, _accepts_control=0, _allow_construction=True, @@ -125,6 +130,7 @@ def test_accessors_raise_when_unrecognized() -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), + name="", _provides_telemetry=999, _accepts_control=999, _allow_construction=True, @@ -141,12 +147,12 @@ def test_accessors_raise_when_unrecognized() -> None: @pytest.mark.parametrize( "name,expected_str", [ - (None, "CID1<_TestElectricalComponent>"), + ("", "CID1<_TestElectricalComponent>"), ("test-component", "CID1<_TestElectricalComponent>:test-component"), ], ids=["no-name", "with-name"], ) -def test_str(name: str | None, expected_str: str) -> None: +def test_str(name: str, expected_str: str) -> None: """Test string representation of an electrical component.""" component = _TestElectricalComponent( id=ElectricalComponentId(1), @@ -170,6 +176,7 @@ def test_operational_at(is_operational: bool) -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), + name="", operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, @@ -195,6 +202,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), + name="", operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_ev_charger.py b/tests/microgrid/electrical_components/test_ev_charger.py index 74ee208..83568c2 100644 --- a/tests/microgrid/electrical_components/test_ev_charger.py +++ b/tests/microgrid/electrical_components/test_ev_charger.py @@ -96,6 +96,7 @@ def test_unspecified_ev_charger_is_problematic( charger = UnspecifiedEvCharger( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -112,6 +113,7 @@ def test_unrecognized_ev_charger_is_problematic( charger = UnrecognizedEvCharger( id=component_id, microgrid_id=microgrid_id, + name="", type=999, _provides_telemetry=True, _accepts_control=True, @@ -132,6 +134,7 @@ def test_recognized_ev_charger_types_are_not_problematic( charger = cls( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/electrical_components/test_grid_connection_point.py b/tests/microgrid/electrical_components/test_grid_connection_point.py index cc85f8c..2bbd70e 100644 --- a/tests/microgrid/electrical_components/test_grid_connection_point.py +++ b/tests/microgrid/electrical_components/test_grid_connection_point.py @@ -73,6 +73,7 @@ def test_creation_without_flag_raises( GridConnectionPoint( id=component_id, microgrid_id=microgrid_id, + name="", rated_fuse_current=0, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_inverter.py b/tests/microgrid/electrical_components/test_inverter.py index 7069e57..3ebfb3f 100644 --- a/tests/microgrid/electrical_components/test_inverter.py +++ b/tests/microgrid/electrical_components/test_inverter.py @@ -96,6 +96,7 @@ def test_unspecified_inverter_is_problematic( inverter = UnspecifiedInverter( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -112,6 +113,7 @@ def test_unrecognized_inverter_is_problematic( inverter = UnrecognizedInverter( id=component_id, microgrid_id=microgrid_id, + name="", type=999, _provides_telemetry=True, _accepts_control=True, @@ -132,6 +134,7 @@ def test_recognized_inverter_types_are_not_problematic( inverter = cls( id=component_id, microgrid_id=microgrid_id, + name="", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index a24b091..f8d7eec 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -118,7 +118,7 @@ def test_is_active_unrecognized() -> None: info = Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), - name=None, + name="", delivery_area=None, location=None, create_time=now, From a45c7558a075661114a6da8c7072b5095d9e37f9 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 6 Jul 2026 12:47:45 +0000 Subject: [PATCH 4/4] Stop reporting empty electrical component names as an issue Now that `ElectricalComponent.name` is `str` and represents the absence of a name as `""`, having no name is not an issue worth flagging. Drop the `name is empty` minor issue in `_electrical_component_base_from_proto_with_issues` and inline `message.name` in the resulting `_ElectricalComponentBaseData`. Signed-off-by: Leandro Lucarella --- .../proto/v1alpha8/_electrical_component.py | 6 +----- .../proto/v1alpha8/test_electrical_component_base.py | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index e2c853d..ecc4895 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -928,10 +928,6 @@ def _electrical_component_base_from_proto_with_issues( component_id = ElectricalComponentId(message.id) microgrid_id = MicrogridId(message.microgrid_id) - name = message.name - if not name: - minor_issues.append("name is empty") - model = message.model or None if model is None: minor_issues.append("model is empty") @@ -979,7 +975,7 @@ def _electrical_component_base_from_proto_with_issues( return _ElectricalComponentBaseData( component_id, microgrid_id, - name, + message.name, model, category, lifetime, diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 48011b3..dc1e707 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -113,7 +113,6 @@ def test_missing_category_specific_info( assert sorted(major_issues) == sorted(["category is unspecified"]) assert sorted(minor_issues) == sorted( [ - "name is empty", "missing operational lifetime, considering it always operational", ] )