From 085577ea2b33be80b3836759ae97c4c59aec753e Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 16 Jun 2026 10:11:10 +0000 Subject: [PATCH 1/5] Rename `ComponentTypes` to `ElectricalComponentTypes` Rename the generic component union alias so it carries the same `Electrical` prefix as its sibling aliases and keep the converter imports sorted. Signed-off-by: Leandro Lucarella --- .../common/microgrid/electrical_components/__init__.py | 4 ++-- .../client/common/microgrid/electrical_components/_types.py | 4 ++-- .../proto/v1alpha8/_electrical_component.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/__init__.py b/src/frequenz/client/common/microgrid/electrical_components/__init__.py index 66875563..d1e75c50 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/__init__.py +++ b/src/frequenz/client/common/microgrid/electrical_components/__init__.py @@ -56,7 +56,7 @@ from ._state_code import ElectricalComponentStateCode from ._steam_boiler import SteamBoiler from ._types import ( - ComponentTypes, + ElectricalComponentTypes, ProblematicElectricalComponentTypes, UnrecognizedElectricalComponentTypes, UnspecifiedElectricalComponentTypes, @@ -71,7 +71,6 @@ "BatteryTypes", "Breaker", "Chp", - "ComponentTypes", "Converter", "CryptoMiner", "DcEvCharger", @@ -81,6 +80,7 @@ "ElectricalComponentDiagnosticCode", "ElectricalComponentId", "ElectricalComponentStateCode", + "ElectricalComponentTypes", "Electrolyzer", "EvCharger", "EvChargerType", diff --git a/src/frequenz/client/common/microgrid/electrical_components/_types.py b/src/frequenz/client/common/microgrid/electrical_components/_types.py index fadb04eb..105b5eae 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_types.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_types.py @@ -48,7 +48,7 @@ ) """All possible electrical component types that have a problem.""" -ComponentTypes: TypeAlias = ( +ElectricalComponentTypes: TypeAlias = ( BatteryTypes | Breaker | Chp @@ -66,4 +66,4 @@ | SteamBoiler | WindTurbine ) -"""All possible component types.""" +"""All possible electrical component types.""" 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 db3ac569..38dc777e 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 @@ -24,12 +24,12 @@ BatteryType, Breaker, Chp, - ComponentTypes, Converter, CryptoMiner, DcEvCharger, ElectricalComponentCategory, ElectricalComponentId, + ElectricalComponentTypes, Electrolyzer, EvChargerType, GridConnectionPoint, @@ -66,7 +66,7 @@ def electrical_component_from_proto( message: electrical_components_pb2.ElectricalComponent, -) -> ComponentTypes: +) -> ElectricalComponentTypes: """Convert a protobuf message to an electrical component instance. Args: @@ -200,7 +200,7 @@ def electrical_component_from_proto_with_issues( *, major_issues: list[str], minor_issues: list[str], -) -> ComponentTypes: +) -> ElectricalComponentTypes: """Convert a protobuf message to an electrical component and collect issues. Args: From ae1f4e3338c3443aa1129974bfd86b31afa49fde Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 16 Jun 2026 10:11:10 +0000 Subject: [PATCH 2/5] Rename connection `source`/`destination` to `*_id` Rename the wrapper-facing connection endpoints while keeping the protobuf field reads and local converter variables unchanged. Signed-off-by: Leandro Lucarella --- .../_electrical_component_connection.py | 8 ++--- .../_electrical_component_connection.py | 4 +-- .../test_electrical_component_connection.py | 8 ++--- .../test_electrical_component_connection.py | 36 +++++++++---------- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py index 44b943d4..f73ff3f4 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py @@ -37,14 +37,14 @@ class ElectricalComponentConnection: when and how the microgrid infrastructure has been modified. """ - source: ElectricalComponentId + source_id: ElectricalComponentId """The unique identifier of the electrical component where the connection originates. This is aligned with the direction of current flow away from the grid connection point, or in case of islands, away from the islanding point. """ - destination: ElectricalComponentId + destination_id: ElectricalComponentId """The unique ID of the electrical component where the connection terminates. This is the electrical component towards which the current flows. @@ -55,7 +55,7 @@ class ElectricalComponentConnection: def __post_init__(self) -> None: """Ensure that the source and destination electrical components are different.""" - if self.source == self.destination: + if self.source_id == self.destination_id: raise ValueError("Source and destination components must be different") def is_operational_at(self, timestamp: datetime) -> bool: @@ -68,4 +68,4 @@ def is_operational_now(self) -> bool: def __str__(self) -> str: """Return a human-readable string representation of this instance.""" - return f"{self.source}->{self.destination}" + return f"{self.source_id}->{self.destination_id}" diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index 3c78edcb..d8273355 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -79,8 +79,8 @@ def electrical_component_connection_from_proto_with_issues( ) return ElectricalComponentConnection( - source=source_component_id, - destination=destination_component_id, + source_id=source_component_id, + destination_id=destination_component_id, operational_lifetime=lifetime, ) diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py index 218dc2bc..3c18553a 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py @@ -76,10 +76,10 @@ def test_success(proto_data: dict[str, Any], expected_minor_issues: list[str]) - assert connection is not None assert not major_issues assert minor_issues == expected_minor_issues - assert connection.source == ElectricalComponentId( + assert connection.source_id == ElectricalComponentId( proto_data["source_electrical_component_id"] ) - assert connection.destination == ElectricalComponentId( + assert connection.destination_id == ElectricalComponentId( proto_data["destination_electrical_component_id"] ) @@ -131,8 +131,8 @@ def test_invalid_lifetime(mock_lifetime_from_proto: Mock) -> None: ) assert connection is not None - assert connection.source == ElectricalComponentId(1) - assert connection.destination == ElectricalComponentId(2) + assert connection.source_id == ElectricalComponentId(1) + assert connection.destination_id == ElectricalComponentId(2) assert major_issues == [ "invalid operational lifetime (Invalid lifetime), considering it as missing " "(i.e. always operational)" diff --git a/tests/microgrid/electrical_components/test_electrical_component_connection.py b/tests/microgrid/electrical_components/test_electrical_component_connection.py index 0f8c2977..920b6a98 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/test_electrical_component_connection.py @@ -20,13 +20,13 @@ def test_creation() -> None: now = datetime.now(timezone.utc) lifetime = Lifetime(start=now) connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=lifetime, ) - assert connection.source == ElectricalComponentId(1) - assert connection.destination == ElectricalComponentId(2) + assert connection.source_id == ElectricalComponentId(1) + assert connection.destination_id == ElectricalComponentId(2) assert connection.operational_lifetime == lifetime @@ -36,14 +36,14 @@ def test_validation() -> None: ValueError, match="Source and destination components must be different" ): ElectricalComponentConnection( - source=ElectricalComponentId(1), destination=ElectricalComponentId(1) + source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(1) ) def test_str() -> None: """Test string representation of ElectricalComponentConnection.""" connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), destination=ElectricalComponentId(2) + source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2) ) assert str(connection) == "CID1->CID2" @@ -52,18 +52,18 @@ def test_equality_and_hash() -> None: """Test equality and hashing of the frozen ElectricalComponentConnection.""" lifetime = Lifetime(start=datetime(2025, 1, 1, tzinfo=timezone.utc)) connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=lifetime, ) same = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=lifetime, ) different = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(3), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(3), operational_lifetime=lifetime, ) @@ -78,8 +78,8 @@ def test_is_operational_at_boundaries() -> None: start = datetime(2025, 1, 1, tzinfo=timezone.utc) end = datetime(2025, 12, 31, tzinfo=timezone.utc) connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=Lifetime(start=start, end=end), ) @@ -103,8 +103,8 @@ def test_is_operational_at(lifetime_active: bool) -> None: mock_lifetime.is_operational_at.return_value = lifetime_active connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=mock_lifetime, ) @@ -128,8 +128,8 @@ def test_is_operational_now(mock_datetime: Mock, lifetime_active: bool) -> None: mock_lifetime.is_operational_at.return_value = lifetime_active connection = ElectricalComponentConnection( - source=ElectricalComponentId(1), - destination=ElectricalComponentId(2), + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), operational_lifetime=mock_lifetime, ) From 8c1418eb4d4c07ee36e204c36c3ff758a41a2baf Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 16 Jun 2026 10:11:16 +0000 Subject: [PATCH 3/5] Rename `Lifetime.start`/`end` to `start_time`/`end_time` Rename the wrapper-facing lifetime attributes to use the `_time` suffix while keeping protobuf timestamp reads unchanged and updating validation, helpers, and tests. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/_lifetime.py | 33 ++++++----- .../common/types/proto/v1alpha8/_lifetime.py | 2 +- .../proto/v1alpha8/conftest.py | 14 +++-- .../test_electrical_component_connection.py | 6 +- tests/types/proto/v1alpha8/test_lifetime.py | 8 +-- tests/types/test_lifetime.py | 56 +++++++++---------- 6 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py index 867b7e5d..2ac1a8fc 100644 --- a/src/frequenz/client/common/types/_lifetime.py +++ b/src/frequenz/client/common/types/_lifetime.py @@ -12,18 +12,18 @@ class Lifetime: """An active operational period of an asset. Warning: - The [`end`][.end] timestamp indicates that the asset has been permanently - removed from service. + The [`end_time`][.end_time] timestamp indicates that the asset has been + permanently removed from service. """ - start: datetime | None = None + start_time: datetime | None = None """The moment when the asset became operationally active. If `None`, the asset is considered to be active in any past moment previous to the - [`end`][..end]. + [`end_time`][..end_time]. """ - end: datetime | None = None + end_time: datetime | None = None """The moment when the asset's operational activity ceased. If `None`, the asset is considered to be active with no plans to be deactivated. @@ -31,21 +31,26 @@ class Lifetime: def __post_init__(self) -> None: """Validate this lifetime.""" - if self.start is not None and self.end is not None and self.start > self.end: + if ( + self.start_time is not None + and self.end_time is not None + and self.start_time > self.end_time + ): raise ValueError( - f"Start ({self.start}) must be before or equal to end ({self.end})" + f"Start ({self.start_time}) must be before or equal to end " + f"({self.end_time})" ) def is_operational_at(self, timestamp: datetime) -> bool: """Check whether this lifetime is active at a specific timestamp.""" - # Handle start time - it's not active if start is in the future - if self.start is not None and self.start > timestamp: + # Handle start time - it's not active if start_time is in the future + if self.start_time is not None and self.start_time > timestamp: return False - # Handle end time - active up to and including end time - if self.end is not None: - return self.end >= timestamp - # self.end is None, and either self.start is None or self.start <= timestamp, - # so it is active at this timestamp + # Handle end time - active up to and including end_time + if self.end_time is not None: + return self.end_time >= timestamp + # self.end_time is None, and either self.start_time is None or + # self.start_time <= timestamp, so it is active at this timestamp return True def is_operational_now(self) -> bool: diff --git a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py index 4d92138d..00462979 100644 --- a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py +++ b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py @@ -23,4 +23,4 @@ def lifetime_from_proto( if message.HasField("end_timestamp") else None ) - return Lifetime(start=start, end=end) + return Lifetime(start_time=start, end_time=end) diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index b96e399a..9bf76ac9 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -27,8 +27,8 @@ from frequenz.client.common.types import Lifetime DEFAULT_LIFETIME = Lifetime( - start=datetime(2020, 1, 1, tzinfo=timezone.utc), - end=datetime(2030, 1, 1, tzinfo=timezone.utc), + start_time=datetime(2020, 1, 1, tzinfo=timezone.utc), + end_time=datetime(2030, 1, 1, tzinfo=timezone.utc), ) DEFAULT_COMPONENT_ID = ElectricalComponentId(42) DEFAULT_MICROGRID_ID = MicrogridId(1) @@ -101,12 +101,14 @@ def base_data_as_proto( ) if base_data.lifetime: lifetime_dict: dict[str, Timestamp] = {} - if base_data.lifetime.start is not None: + if base_data.lifetime.start_time is not None: lifetime_dict["start_timestamp"] = datetime_to_proto( - base_data.lifetime.start + base_data.lifetime.start_time + ) + if base_data.lifetime.end_time is not None: + lifetime_dict["end_timestamp"] = datetime_to_proto( + base_data.lifetime.end_time ) - if base_data.lifetime.end is not None: - lifetime_dict["end_timestamp"] = datetime_to_proto(base_data.lifetime.end) proto.operational_lifetime.CopyFrom(lifetime_pb2.Lifetime(**lifetime_dict)) if base_data.rated_bounds: for metric, bounds in base_data.rated_bounds.items(): diff --git a/tests/microgrid/electrical_components/test_electrical_component_connection.py b/tests/microgrid/electrical_components/test_electrical_component_connection.py index 920b6a98..5e25b764 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/test_electrical_component_connection.py @@ -18,7 +18,7 @@ def test_creation() -> None: """Test basic ElectricalComponentConnection creation and validation.""" now = datetime.now(timezone.utc) - lifetime = Lifetime(start=now) + lifetime = Lifetime(start_time=now) connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), @@ -50,7 +50,7 @@ def test_str() -> None: def test_equality_and_hash() -> None: """Test equality and hashing of the frozen ElectricalComponentConnection.""" - lifetime = Lifetime(start=datetime(2025, 1, 1, tzinfo=timezone.utc)) + lifetime = Lifetime(start_time=datetime(2025, 1, 1, tzinfo=timezone.utc)) connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), @@ -80,7 +80,7 @@ def test_is_operational_at_boundaries() -> None: connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), - operational_lifetime=Lifetime(start=start, end=end), + operational_lifetime=Lifetime(start_time=start, end_time=end), ) before = start - timedelta(seconds=1) diff --git a/tests/types/proto/v1alpha8/test_lifetime.py b/tests/types/proto/v1alpha8/test_lifetime.py index a6a9da0d..f30b8151 100644 --- a/tests/types/proto/v1alpha8/test_lifetime.py +++ b/tests/types/proto/v1alpha8/test_lifetime.py @@ -78,14 +78,14 @@ def test_from_proto( lifetime = lifetime_from_proto(proto) if case.include_start: - assert lifetime.start == now + assert lifetime.start_time == now else: - assert lifetime.start is None + assert lifetime.start_time is None if case.include_end: - assert lifetime.end == future + assert lifetime.end_time == future else: - assert lifetime.end is None + assert lifetime.end_time is None def test_from_proto_rejects_start_after_end(now: datetime, future: datetime) -> None: diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py index 0921839d..f0768872 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/test_lifetime.py @@ -32,10 +32,10 @@ class _LifetimeTestCase: name: str """The description of the test case.""" - start: bool + include_start: bool """Whether to include start time.""" - end: bool + include_end: bool """Whether to include end time.""" expected_start: bool @@ -102,32 +102,32 @@ def future(present: datetime) -> datetime: [ _LifetimeTestCase( name="full", - start=True, - end=True, + include_start=True, + include_end=True, expected_start=True, expected_end=True, expected_operational=True, ), _LifetimeTestCase( name="only_start", - start=True, - end=False, + include_start=True, + include_end=False, expected_start=True, expected_end=False, expected_operational=True, ), _LifetimeTestCase( name="only_end", - start=False, - end=True, + include_start=False, + include_end=True, expected_start=False, expected_end=True, expected_operational=True, ), _LifetimeTestCase( name="no_dates", - start=False, - end=False, + include_start=False, + include_end=False, expected_start=False, expected_end=False, expected_operational=True, @@ -138,15 +138,15 @@ def future(present: datetime) -> datetime: def test_creation(present: datetime, future: datetime, case: _LifetimeTestCase) -> None: """Test creating Lifetime instances with various parameters.""" lifetime = Lifetime( - start=present if case.start else None, - end=future if case.end else None, + start_time=present if case.include_start else None, + end_time=future if case.include_end else None, ) - assert (lifetime.start is not None) == case.expected_start + assert (lifetime.start_time is not None) == case.expected_start if case.expected_start: - assert lifetime.start == present - assert (lifetime.end is not None) == case.expected_end + assert lifetime.start_time == present + assert (lifetime.end_time is not None) == case.expected_end if case.expected_end: - assert lifetime.end == future + assert lifetime.end_time == future assert lifetime.is_operational_now() == case.expected_operational @@ -185,28 +185,28 @@ def test_validation( with pytest.raises( ValueError, match=r"Start \(.*\) must be before or equal to end \(.*\)" ): - Lifetime(start=start_time, end=end_time) + Lifetime(start_time=start_time, end_time=end_time) else: - lifetime = Lifetime(start=start_time, end=end_time) + lifetime = Lifetime(start_time=start_time, end_time=end_time) # Verify the timestamps are set correctly - assert lifetime.start == start_time - assert lifetime.end == end_time + assert lifetime.start_time == start_time + assert lifetime.end_time == end_time def test_equal_start_and_end_is_valid(present: datetime) -> None: """Test that a Lifetime with the same start and end time is valid.""" - lifetime = Lifetime(start=present, end=present) + lifetime = Lifetime(start_time=present, end_time=present) - assert lifetime.start == present - assert lifetime.end == present + assert lifetime.start_time == present + assert lifetime.end_time == present assert lifetime.is_operational_at(present) def test_equality_and_hashing(present: datetime, future: datetime) -> None: """Test that Lifetime objects support equality and hashing.""" - lifetime1 = Lifetime(start=present, end=future) - lifetime2 = Lifetime(start=present, end=future) - lifetime3 = Lifetime(start=present, end=None) + lifetime1 = Lifetime(start_time=present, end_time=future) + lifetime2 = Lifetime(start_time=present, end_time=future) + lifetime3 = Lifetime(start_time=present, end_time=None) assert lifetime1 == lifetime2 assert lifetime1 != lifetime3 @@ -285,7 +285,7 @@ def test_active_property( None: None, }[case.end_type] - lifetime = Lifetime(start=start_time, end=end_time) + lifetime = Lifetime(start_time=start_time, end_time=end_time) assert lifetime.is_operational_at(present) == case.expected_operational @@ -311,7 +311,7 @@ def test_active_at_with_fixed_lifetime( case: _FixedLifetimeTestCase, ) -> None: """Test active_at with different timestamps for a fixed lifetime period.""" - lifetime = Lifetime(start=past, end=future) + lifetime = Lifetime(start_time=past, end_time=future) test_time = { _Time.PAST: past, _Time.PRESENT: present, From 8e59a2eab775c6123bca9b782b30dfa061cab920 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 16 Jun 2026 10:11:20 +0000 Subject: [PATCH 4/5] Rename `Microgrid.create_timestamp` to `create_time` Adopt the `_time` wrapper naming convention while keeping the converter pointed at the protobuf `create_timestamp` field. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/microgrid/_microgrid.py | 2 +- .../common/microgrid/proto/v1alpha8/_microgrid.py | 3 ++- tests/microgrid/proto/v1alpha8/test_microgrid.py | 2 +- tests/microgrid/test_microgrid.py | 12 ++++++------ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index fc2874e5..d6e49be4 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -66,7 +66,7 @@ class Microgrid: status: MicrogridStatus | int """The current status of the microgrid.""" - create_timestamp: datetime.datetime + create_time: datetime.datetime """The UTC timestamp indicating when the microgrid was initially created.""" @cached_property diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index 37d7c905..e3b8aa59 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -95,6 +95,7 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: message, ) + # The wrapper uses create_time, but the protobuf field remains create_timestamp. return Microgrid( id=MicrogridId(message.id), enterprise_id=EnterpriseId(message.enterprise_id), @@ -102,5 +103,5 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: delivery_area=delivery_area, location=location, status=status, - create_timestamp=datetime_from_proto(message.create_timestamp), + 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 63b0761d..84fb64bf 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -191,7 +191,7 @@ def test_from_proto( # Verify the result assert info.id == MicrogridId(1234) assert info.enterprise_id == EnterpriseId(5678) - assert info.create_timestamp == now + assert info.create_time == now if case.has_name: assert info.name == "Test Grid" diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index 012c04c4..afca27bd 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -29,7 +29,7 @@ def test_creation() -> None: ), location=Location(latitude=52.52, longitude=13.405, country_code="DE"), status=MicrogridStatus.ACTIVE, - create_timestamp=now, + create_time=now, ) assert info.id == MicrogridId(1234) @@ -45,7 +45,7 @@ def test_creation() -> None: assert info.location.longitude == pytest.approx(13.405) assert info.location.country_code == "DE" assert info.status == MicrogridStatus.ACTIVE - assert info.create_timestamp == now + assert info.create_time == now assert info.is_active is True @@ -59,7 +59,7 @@ def test_creation_without_optionals() -> None: delivery_area=None, location=None, status=MicrogridStatus.ACTIVE, - create_timestamp=now, + create_time=now, ) assert info.id == MicrogridId(1234) @@ -68,7 +68,7 @@ def test_creation_without_optionals() -> None: assert info.delivery_area is None assert info.location is None assert info.status == MicrogridStatus.ACTIVE - assert info.create_timestamp == now + assert info.create_time == now assert info.is_active is True @@ -90,7 +90,7 @@ def test_is_active_property(status: MicrogridStatus, expected_active: bool) -> N delivery_area=None, location=None, status=status, - create_timestamp=now, + create_time=now, ) assert info.is_active is expected_active @@ -113,6 +113,6 @@ def test_str(name: str | None, expected_str: str) -> None: delivery_area=None, location=None, status=MicrogridStatus.ACTIVE, - create_timestamp=now, + create_time=now, ) assert str(info) == expected_str From b022c61ba4a5bbdf4157712fab48356617966621 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 16 Jun 2026 10:12:05 +0000 Subject: [PATCH 5/5] Document wrapper naming conventions Record the field naming and docstring rules in CONTRIBUTING.md as the source of truth and point the code-pattern guide to it. Signed-off-by: Leandro Lucarella --- CONTRIBUTING.md | 24 ++++++++++++++++++++++++ src/frequenz/client/common/AGENTS.md | 1 + 2 files changed, 25 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b2497b56..d11ab596 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -115,6 +115,30 @@ That said, if you want to test the actual website in **your fork**, you can always use `mike deploy --push --remote your-fork-remote`, and then access the GitHub pages produced for your fork. +## Wrapper conventions + +### Field names and docstrings + +The following rules apply when wrapping protobuf messages into idiomatic Python +types: + +1. **Alignment**: By default, wrapper field names match the protobuf field names + unless a clear Pythonic improvement preserves or clarifies semantics. +2. **IDs**: Always keep the `_id` suffix for fields representing identifiers + (e.g., `id`, `microgrid_id`, `enterprise_id`, `source_id`, `destination_id`). +3. **Redundancy**: You may drop a redundant or long entity prefix while keeping + `_id`. For example, `source_electrical_component_id` becomes `source_id`. +4. **Time fields**: Use the `_time` suffix for both protobuf `_time` and + `_timestamp` fields. Never drop the suffix, as bare names like `start` or + `create` can be read as verbs or actions. For example, `create_timestamp` + becomes `create_time` and `start_timestamp` becomes `start_time`. +5. **Values**: You may drop the `_value` suffix inside a class ending in `Value` + when the remaining name remains clear. For example, `avg`, `min`, `max`, and + `raw` in `AggregatedMetricValue`. +6. **Docstrings**: Docstrings may be shorter or more Pythonic than the protobuf + comments, but they must not contradict, narrow, broaden, or operationally + reinterpret the protobuf semantics. + ## Releasing These are the steps to create a new release: diff --git a/src/frequenz/client/common/AGENTS.md b/src/frequenz/client/common/AGENTS.md index f7b6a25f..c8981b55 100644 --- a/src/frequenz/client/common/AGENTS.md +++ b/src/frequenz/client/common/AGENTS.md @@ -28,6 +28,7 @@ for downstream users, they don't wrap protobuf messages. ## CORE RULES +- **Wrapper field names and docstrings follow the rules in [CONTRIBUTING.md](../../../../CONTRIBUTING.md).** - **Public symbols live in `_name.py`, exported via the package `__init__.py`.** External importers never use the underscore module path. - **Internal cross-module imports are ALWAYS relative and use the real symbol