From 3ee9719aa52d6bfe6438ee140c7463dbdadae62c Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 8 Jul 2026 10:01:21 +0000 Subject: [PATCH 1/5] Add invalid latitude/longitude/country code exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add three new subclasses of `InvalidAttributeError`: * `InvalidLatitudeError` — the raw `float` was outside `[-90, 90]` * `InvalidLongitudeError` — the raw `float` was outside `[-180, 180]` * `InvalidCountryCodeError` — the raw `str` was not exactly 2 characters Each stores the offending value on `.value` (`float` for the coordinate errors, `str` for the country code error). These will be raised by the upcoming `Location.get_latitude()`, `.get_longitude()` and `.get_country_code()` accessors (next commits), where the low-level fields on `Location` may carry the raw invalid data that came off the wire. Also split `test_location.py` into multiple files as it will grow. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 10 +- src/frequenz/client/common/types/_location.py | 122 ++++++++++++++++++ tests/types/_location/__init__.py | 4 + .../test_invalid_country_code_error.py | 39 ++++++ .../_location/test_invalid_latitude_error.py | 37 ++++++ .../_location/test_invalid_longitude_error.py | 37 ++++++ tests/types/{ => _location}/test_location.py | 0 7 files changed, 248 insertions(+), 1 deletion(-) create mode 100644 tests/types/_location/__init__.py create mode 100644 tests/types/_location/test_invalid_country_code_error.py create mode 100644 tests/types/_location/test_invalid_latitude_error.py create mode 100644 tests/types/_location/test_invalid_longitude_error.py rename tests/types/{ => _location}/test_location.py (100%) diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index f07ef000..35d1a9ca 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -4,9 +4,17 @@ """Common types.""" from ._lifetime import Lifetime -from ._location import Location +from ._location import ( + InvalidCountryCodeError, + InvalidLatitudeError, + InvalidLongitudeError, + Location, +) __all__ = [ + "InvalidCountryCodeError", + "InvalidLatitudeError", + "InvalidLongitudeError", "Lifetime", "Location", ] diff --git a/src/frequenz/client/common/types/_location.py b/src/frequenz/client/common/types/_location.py index 307d16a0..f4d7852c 100644 --- a/src/frequenz/client/common/types/_location.py +++ b/src/frequenz/client/common/types/_location.py @@ -5,6 +5,128 @@ from dataclasses import dataclass +from .._exception import InvalidAttributeError + + +class InvalidLatitudeError(InvalidAttributeError): + """Raised when a semantic accessor sees a latitude outside `[-90, 90]`. + + A well-formed latitude lies in the closed interval `[-90, 90]`. The raw + out-of-range float is available as `value`. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + value: float, + message: str | None = None, + ) -> None: + """Initialize this error. + + Args: + instance: The object instance that had the invalid latitude. + attr_name: The name of the attribute that had the invalid latitude. + value: The out-of-range latitude value. + message: A custom error message. If `None`, a default message + mentioning the invalid value is used. + """ + self.value: float = value + """The out-of-range latitude value.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid latitude {value!r} for attribute {attr_name!r} in " + f"{instance}; must be in [-90, 90]" + ), + ) + + +class InvalidLongitudeError(InvalidAttributeError): + """Raised when a semantic accessor sees a longitude outside `[-180, 180]`. + + A well-formed longitude lies in the closed interval `[-180, 180]`. The raw + out-of-range float is available as `value`. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + value: float, + message: str | None = None, + ) -> None: + """Initialize this error. + + Args: + instance: The object instance that had the invalid longitude. + attr_name: The name of the attribute that had the invalid longitude. + value: The out-of-range longitude value. + message: A custom error message. If `None`, a default message + mentioning the invalid value is used. + """ + self.value: float = value + """The out-of-range longitude value.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid longitude {value!r} for attribute {attr_name!r} in " + f"{instance}; must be in [-180, 180]" + ), + ) + + +class InvalidCountryCodeError(InvalidAttributeError): + """Raised when a semantic accessor sees a country code of length other than 2. + + A well-formed country code is an ISO 3166-1 Alpha-2 string, so it must be + exactly 2 characters long. The raw string is available as `value`. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + value: str, + message: str | None = None, + ) -> None: + """Initialize this error. + + Args: + instance: The object instance that had the invalid country code. + attr_name: The name of the attribute that had the invalid country code. + value: The invalid country code string. + message: A custom error message. If `None`, a default message + mentioning the invalid value is used. + """ + self.value: str = value + """The invalid country code string.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid country code {value!r} for attribute {attr_name!r} in " + f"{instance}; must be exactly 2 characters" + ), + ) + @dataclass(frozen=True, kw_only=True) class Location: diff --git a/tests/types/_location/__init__.py b/tests/types/_location/__init__.py new file mode 100644 index 00000000..2f2a6ca9 --- /dev/null +++ b/tests/types/_location/__init__.py @@ -0,0 +1,4 @@ +# License: MIT +# Copyright © 2025 Frequenz Energy-as-a-Service GmbH + +"""Tests for the Location type and its Invalid* wrapper types.""" diff --git a/tests/types/_location/test_invalid_country_code_error.py b/tests/types/_location/test_invalid_country_code_error.py new file mode 100644 index 00000000..a5f63a90 --- /dev/null +++ b/tests/types/_location/test_invalid_country_code_error.py @@ -0,0 +1,39 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidCountryCodeError`.""" + +from frequenz.client.common import ClientCommonError, InvalidAttributeError +from frequenz.client.common.types import InvalidCountryCodeError + + +def test_inherits_invalid_attribute_error() -> None: + """`InvalidCountryCodeError` inherits `InvalidAttributeError` (and thus `ValueError`).""" + assert issubclass(InvalidCountryCodeError, InvalidAttributeError) + assert issubclass(InvalidCountryCodeError, ClientCommonError) + assert issubclass(InvalidCountryCodeError, ValueError) + + +def test_stores_instance_attr_name_and_value() -> None: + """`InvalidCountryCodeError` stores `instance`, `attr_name`, and `value` as attributes.""" + instance = object() + error = InvalidCountryCodeError(instance, "country_code", "DEU") + assert error.instance is instance + assert error.attr_name == "country_code" + assert error.value == "DEU" + + +def test_default_message() -> None: + """The default message follows the `invalid country code ...` template.""" + assert ( + str(InvalidCountryCodeError("some-instance", "country_code", "DEU")) + == "invalid country code 'DEU' for attribute 'country_code' in some-instance; " + "must be exactly 2 characters" + ) + + +def test_custom_message_replaces_the_default() -> None: + """A custom message replaces the default entirely.""" + assert ( + str(InvalidCountryCodeError("i", "a", "DEU", "explicit msg")) == "explicit msg" + ) diff --git a/tests/types/_location/test_invalid_latitude_error.py b/tests/types/_location/test_invalid_latitude_error.py new file mode 100644 index 00000000..17719c49 --- /dev/null +++ b/tests/types/_location/test_invalid_latitude_error.py @@ -0,0 +1,37 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidLatitudeError`.""" + +from frequenz.client.common import ClientCommonError, InvalidAttributeError +from frequenz.client.common.types import InvalidLatitudeError + + +def test_inherits_invalid_attribute_error() -> None: + """`InvalidLatitudeError` inherits `InvalidAttributeError` (and thus `ValueError`).""" + assert issubclass(InvalidLatitudeError, InvalidAttributeError) + assert issubclass(InvalidLatitudeError, ClientCommonError) + assert issubclass(InvalidLatitudeError, ValueError) + + +def test_stores_instance_attr_name_and_value() -> None: + """`InvalidLatitudeError` stores `instance`, `attr_name`, and `value` as attributes.""" + instance = object() + error = InvalidLatitudeError(instance, "latitude", 91.0) + assert error.instance is instance + assert error.attr_name == "latitude" + assert error.value == 91.0 + + +def test_default_message() -> None: + """The default message follows the `invalid latitude ...` template.""" + assert ( + str(InvalidLatitudeError("some-instance", "latitude", 91.0)) + == "invalid latitude 91.0 for attribute 'latitude' in some-instance; " + "must be in [-90, 90]" + ) + + +def test_custom_message_replaces_the_default() -> None: + """A custom message replaces the default entirely.""" + assert str(InvalidLatitudeError("i", "a", 91.0, "explicit msg")) == "explicit msg" diff --git a/tests/types/_location/test_invalid_longitude_error.py b/tests/types/_location/test_invalid_longitude_error.py new file mode 100644 index 00000000..085e7fc6 --- /dev/null +++ b/tests/types/_location/test_invalid_longitude_error.py @@ -0,0 +1,37 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidLongitudeError`.""" + +from frequenz.client.common import ClientCommonError, InvalidAttributeError +from frequenz.client.common.types import InvalidLongitudeError + + +def test_inherits_invalid_attribute_error() -> None: + """`InvalidLongitudeError` inherits `InvalidAttributeError` (and thus `ValueError`).""" + assert issubclass(InvalidLongitudeError, InvalidAttributeError) + assert issubclass(InvalidLongitudeError, ClientCommonError) + assert issubclass(InvalidLongitudeError, ValueError) + + +def test_stores_instance_attr_name_and_value() -> None: + """`InvalidLongitudeError` stores `instance`, `attr_name`, and `value` as attributes.""" + instance = object() + error = InvalidLongitudeError(instance, "longitude", 181.0) + assert error.instance is instance + assert error.attr_name == "longitude" + assert error.value == 181.0 + + +def test_default_message() -> None: + """The default message follows the `invalid longitude ...` template.""" + assert ( + str(InvalidLongitudeError("some-instance", "longitude", 181.0)) + == "invalid longitude 181.0 for attribute 'longitude' in some-instance; " + "must be in [-180, 180]" + ) + + +def test_custom_message_replaces_the_default() -> None: + """A custom message replaces the default entirely.""" + assert str(InvalidLongitudeError("i", "a", 181.0, "explicit msg")) == "explicit msg" diff --git a/tests/types/test_location.py b/tests/types/_location/test_location.py similarity index 100% rename from tests/types/test_location.py rename to tests/types/_location/test_location.py From bde212797c6b6efb6e948c496d8e5fce2936921e Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 8 Jul 2026 11:21:10 +0000 Subject: [PATCH 2/5] Add `Invalid{Latitude,Longitude,CountryCode` classes These classes will be used to store invalid location attributes explicitly, so they can still be inspected, but can't be used accidentally. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 6 +++ src/frequenz/client/common/types/_location.py | 46 +++++++++++++++++++ .../_location/test_invalid_country_code.py | 24 ++++++++++ .../types/_location/test_invalid_latitude.py | 26 +++++++++++ .../types/_location/test_invalid_longitude.py | 26 +++++++++++ 5 files changed, 128 insertions(+) create mode 100644 tests/types/_location/test_invalid_country_code.py create mode 100644 tests/types/_location/test_invalid_latitude.py create mode 100644 tests/types/_location/test_invalid_longitude.py diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index 35d1a9ca..9ce60ed1 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -5,15 +5,21 @@ from ._lifetime import Lifetime from ._location import ( + InvalidCountryCode, InvalidCountryCodeError, + InvalidLatitude, InvalidLatitudeError, + InvalidLongitude, InvalidLongitudeError, Location, ) __all__ = [ + "InvalidCountryCode", "InvalidCountryCodeError", + "InvalidLatitude", "InvalidLatitudeError", + "InvalidLongitude", "InvalidLongitudeError", "Lifetime", "Location", diff --git a/src/frequenz/client/common/types/_location.py b/src/frequenz/client/common/types/_location.py index f4d7852c..8368b635 100644 --- a/src/frequenz/client/common/types/_location.py +++ b/src/frequenz/client/common/types/_location.py @@ -128,6 +128,52 @@ def __init__( ) +@dataclass(frozen=True, kw_only=True) +class InvalidLatitude: + """A latitude value that fails the invariant of `[-90, 90]`. + + Wraps a raw wire latitude that fell outside the well-formed range. + """ + + value: float + """The raw out-of-range latitude value.""" + + def __str__(self) -> str: + """Return a compact representation flagging this as an invalid value.""" + return f"" + + +@dataclass(frozen=True, kw_only=True) +class InvalidLongitude: + """A longitude value that fails the invariant of `[-180, 180]`. + + Wraps a raw wire longitude that fell outside the well-formed range. + """ + + value: float + """The raw out-of-range longitude value.""" + + def __str__(self) -> str: + """Return a compact representation flagging this as an invalid value.""" + return f"" + + +@dataclass(frozen=True, kw_only=True) +class InvalidCountryCode: + """A country code that fails the invariant of exactly 2 characters. + + Wraps a raw wire country code that is set but not exactly 2 characters + long. + """ + + value: str + """The raw invalid country code.""" + + def __str__(self) -> str: + """Return a compact representation flagging this as an invalid value.""" + return f"" + + @dataclass(frozen=True, kw_only=True) class Location: """A pair of geographical co-ordinates, representing the location of a place.""" diff --git a/tests/types/_location/test_invalid_country_code.py b/tests/types/_location/test_invalid_country_code.py new file mode 100644 index 00000000..daae2070 --- /dev/null +++ b/tests/types/_location/test_invalid_country_code.py @@ -0,0 +1,24 @@ +# License: MIT +# Copyright © 2025 Frequenz Energy-as-a-Service GmbH + +"""Tests for the InvalidCountryCode wrapper type.""" + +from frequenz.client.common.types import InvalidCountryCode + + +def test_stores_value() -> None: + """`InvalidCountryCode` stores the raw value verbatim.""" + assert InvalidCountryCode(value="DEU").value == "DEU" + + +def test_equality() -> None: + """Two `InvalidCountryCode` with the same value are equal and hash the same.""" + a = InvalidCountryCode(value="DEU") + b = InvalidCountryCode(value="DEU") + assert a == b + assert hash(a) == hash(b) + + +def test_str() -> None: + """`InvalidCountryCode.__str__` renders with a compact invalid marker.""" + assert str(InvalidCountryCode(value="DEU")) == "" diff --git a/tests/types/_location/test_invalid_latitude.py b/tests/types/_location/test_invalid_latitude.py new file mode 100644 index 00000000..5302e298 --- /dev/null +++ b/tests/types/_location/test_invalid_latitude.py @@ -0,0 +1,26 @@ +# License: MIT +# Copyright © 2025 Frequenz Energy-as-a-Service GmbH + +"""Tests for the InvalidLatitude wrapper type.""" + +import pytest + +from frequenz.client.common.types import InvalidLatitude + + +def test_stores_value() -> None: + """`InvalidLatitude` stores the raw value verbatim.""" + assert InvalidLatitude(value=91.0).value == pytest.approx(91.0) + + +def test_equality() -> None: + """Two `InvalidLatitude` with the same value are equal and hash the same.""" + a = InvalidLatitude(value=91.0) + b = InvalidLatitude(value=91.0) + assert a == b + assert hash(a) == hash(b) + + +def test_str() -> None: + """`InvalidLatitude.__str__` renders with a compact invalid marker.""" + assert str(InvalidLatitude(value=91.0)) == "" diff --git a/tests/types/_location/test_invalid_longitude.py b/tests/types/_location/test_invalid_longitude.py new file mode 100644 index 00000000..1596f030 --- /dev/null +++ b/tests/types/_location/test_invalid_longitude.py @@ -0,0 +1,26 @@ +# License: MIT +# Copyright © 2025 Frequenz Energy-as-a-Service GmbH + +"""Tests for the InvalidLongitude wrapper type.""" + +import pytest + +from frequenz.client.common.types import InvalidLongitude + + +def test_stores_value() -> None: + """`InvalidLongitude` stores the raw value verbatim.""" + assert InvalidLongitude(value=181.0).value == pytest.approx(181.0) + + +def test_equality() -> None: + """Two `InvalidLongitude` with the same value are equal and hash the same.""" + a = InvalidLongitude(value=181.0) + b = InvalidLongitude(value=181.0) + assert a == b + assert hash(a) == hash(b) + + +def test_str() -> None: + """`InvalidLongitude.__str__` renders with a compact invalid marker.""" + assert str(InvalidLongitude(value=181.0)) == "" From 730fb9319b5cfeb3e1810f1f1b035e1897101d7a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 9 Jul 2026 12:26:05 +0200 Subject: [PATCH 3/5] Encode validity in `Location` attributes Use the new invalid types for typing `Location` attributes, so invalid values are only accepted while using the wrapper type. This avoids accidental usage of invalid values, and accidental construction of invalid locations, while still allowing for inspection and construction when invalid values are explicitly requested. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/_location.py | 98 ++++++++-- .../common/types/proto/v1alpha8/_location.py | 67 ++++--- tests/microgrid/test_microgrid.py | 2 - tests/types/_location/test_location.py | 185 ++++++++---------- tests/types/proto/v1alpha8/test_location.py | 101 ++++++---- 5 files changed, 254 insertions(+), 199 deletions(-) diff --git a/src/frequenz/client/common/types/_location.py b/src/frequenz/client/common/types/_location.py index 8368b635..0957166d 100644 --- a/src/frequenz/client/common/types/_location.py +++ b/src/frequenz/client/common/types/_location.py @@ -176,34 +176,94 @@ def __str__(self) -> str: @dataclass(frozen=True, kw_only=True) class Location: - """A pair of geographical co-ordinates, representing the location of a place.""" + """A location's information. + + Instances carry the raw wire values of a protobuf `Location` message. + Invalid or absent field values are expressed in the type system: + [`latitude`][.latitude] and [`longitude`][.longitude] may be + [`InvalidLatitude`][..InvalidLatitude] or + [`InvalidLongitude`][..InvalidLongitude]; + [`country_code`][.country_code] may be + [`InvalidCountryCode`][..InvalidCountryCode] or `None` when the field + was unset on the wire. Users can pattern-match on the fields directly. + + Constructing a `Location` with a plain `float` or `str` that violates + its invariant raises `ValueError`; use the corresponding `Invalid*` + type to represent an out-of-invariant wire value. + """ + + latitude: float | InvalidLatitude + """The latitude. + + A plain `float` when well-formed (in `[-90, 90]`); an + [`InvalidLatitude`][...InvalidLatitude] wrapper when the wire delivered + an out-of-range value. + """ + + longitude: float | InvalidLongitude + """The longitude. - latitude: float | None - """The latitude, ranging from -90 (South) to 90 (North).""" + A plain `float` when well-formed (in `[-180, 180]`); an + [`InvalidLongitude`][...InvalidLongitude] wrapper when the wire + delivered an out-of-range value. + """ - longitude: float | None - """The longitude, ranging from -180 (West) to 180 (East).""" + country_code: str | InvalidCountryCode | None + """The country code. - country_code: str | None - """The country code in ISO 3166-1 Alpha 2 format.""" + A plain `str` (exactly 2 characters, ISO 3166-1 Alpha-2) when + well-formed; an [`InvalidCountryCode`][...InvalidCountryCode] wrapper + when the wire delivered a non-empty string of a different length; + `None` when the field was unset on the wire (an empty string on the + wire is normalized to `None` by the converter). + """ def __post_init__(self) -> None: - """Validate latitude and longitude are within their respective ranges.""" - if self.latitude is not None and not -90.0 <= self.latitude <= 90.0: + """Enforce that plain (unwrapped) fields respect their invariants. + + Raises: + ValueError: If `latitude` is a plain `float` outside `[-90, 90]`; + if `longitude` is a plain `float` outside `[-180, 180]`; or + if `country_code` is a plain `str` not exactly 2 characters + long. To represent an invalid wire value, wrap it in the + corresponding `Invalid*` type. + """ + if not isinstance(self.latitude, InvalidLatitude) and not ( + -90.0 <= self.latitude <= 90.0 + ): + raise ValueError( + f"latitude {self.latitude!r} is outside [-90, 90]; wrap in " + "InvalidLatitude to represent an invalid wire value" + ) + if not isinstance(self.longitude, InvalidLongitude) and not ( + -180.0 <= self.longitude <= 180.0 + ): raise ValueError( - f"latitude must be in the range [-90, 90], got {self.latitude!r}" + f"longitude {self.longitude!r} is outside [-180, 180]; wrap " + "in InvalidLongitude to represent an invalid wire value" ) - if self.longitude is not None and not -180.0 <= self.longitude <= 180.0: + if ( + self.country_code is not None + and not isinstance(self.country_code, InvalidCountryCode) + and len(self.country_code) != 2 + ): raise ValueError( - f"longitude must be in the range [-180, 180], got {self.longitude!r}" + f"country_code {self.country_code!r} is not exactly 2 " + "characters; wrap in InvalidCountryCode to represent an " + "invalid wire value" ) def __str__(self) -> str: """Return the short string representation of this instance.""" - country = self.country_code or "" - lat = f"{self.latitude:.2f}" if self.latitude is not None else "?" - lon = f"{self.longitude:.2f}" if self.longitude is not None else "?" - coordinates = "" - if self.latitude is not None or self.longitude is not None: - coordinates = f":({lat}, {lon})" - return f"{country}{coordinates}" + country = self.country_code or "" + lat = ( + str(self.latitude) + if isinstance(self.latitude, InvalidLatitude) + else f"{self.latitude:.2f}" + ) + lon = ( + str(self.longitude) + if isinstance(self.longitude, InvalidLongitude) + else f"{self.longitude:.2f}" + ) + return f"{country}({lat},{lon})" diff --git a/src/frequenz/client/common/types/proto/v1alpha8/_location.py b/src/frequenz/client/common/types/proto/v1alpha8/_location.py index 87c7500a..9c6c3bcc 100644 --- a/src/frequenz/client/common/types/proto/v1alpha8/_location.py +++ b/src/frequenz/client/common/types/proto/v1alpha8/_location.py @@ -3,45 +3,54 @@ """Loading of Location objects from protobuf messages.""" -import logging - from frequenz.api.common.v1alpha8.types import location_pb2 -from ..._location import Location - -_logger = logging.getLogger(__name__) +from ..._location import ( + InvalidCountryCode, + InvalidLatitude, + InvalidLongitude, + Location, +) def location_from_proto(message: location_pb2.Location) -> Location: - """Convert a protobuf location message to a location object. + """Convert a protobuf message to a [`Location`][....Location] object. + + The returned instance carries the raw wire values with the following + normalization applied: + + * Latitude and longitude are wrapped in + [`InvalidLatitude`][....InvalidLatitude] and + [`InvalidLongitude`][....InvalidLongitude] respectively when they + fall outside their well-formed ranges. + * An empty `country_code` on the wire is normalized to `None`; a + non-empty `country_code` that is not exactly 2 characters long is + wrapped in [`InvalidCountryCode`][....InvalidCountryCode]. + + Use the `get_*()` accessors on the returned instance to obtain + validated values or a clear `InvalidAttributeError` subclass. Args: message: The protobuf message to convert. Returns: - The resulting location object. + The resulting [`Location`][....Location] object. """ - issues: list[str] = [] - - latitude: float | None = message.latitude if -90 <= message.latitude <= 90 else None - if latitude is None: - issues.append("latitude out of range [-90, 90]") - - longitude: float | None = ( - message.longitude if -180 <= message.longitude <= 180 else None + latitude: float | InvalidLatitude = ( + message.latitude + if -90.0 <= message.latitude <= 90.0 + else InvalidLatitude(value=message.latitude) ) - if longitude is None: - issues.append("longitude out of range [-180, 180]") - - country_code = message.country_code or None - if country_code is None: - issues.append("country code is empty") - - if issues: - _logger.warning( - "Found issues in location: %s | Protobuf message:\n%s", - ", ".join(issues), - message, - ) - + longitude: float | InvalidLongitude = ( + message.longitude + if -180.0 <= message.longitude <= 180.0 + else InvalidLongitude(value=message.longitude) + ) + country_code: str | InvalidCountryCode | None + if not message.country_code: + country_code = None + elif len(message.country_code) == 2: + country_code = message.country_code + else: + country_code = InvalidCountryCode(value=message.country_code) return Location(latitude=latitude, longitude=longitude, country_code=country_code) diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index 38fe1a32..2bd22457 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -46,9 +46,7 @@ def test_creation() -> None: assert info.delivery_area.code == "DE123" assert info.delivery_area.code_type == EnergyMarketCodeType.EUROPE_EIC assert info.location is not None - assert info.location.latitude is not None assert info.location.latitude == pytest.approx(52.52) - assert info.location.longitude is not None assert info.location.longitude == pytest.approx(13.405) assert info.location.country_code == "DE" assert info.create_time == now diff --git a/tests/types/_location/test_location.py b/tests/types/_location/test_location.py index 8949a446..13022d73 100644 --- a/tests/types/_location/test_location.py +++ b/tests/types/_location/test_location.py @@ -1,133 +1,106 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for the microgrid metadata types.""" +"""Tests for the Location type.""" +import dataclasses import math import pytest -from frequenz.client.common.types import Location +from frequenz.client.common.types import ( + InvalidCountryCode, + InvalidLatitude, + InvalidLongitude, + Location, +) +# ============================================================ +# Construction +# ============================================================ -@pytest.mark.parametrize("latitude", [None, 52.52], ids=str) -@pytest.mark.parametrize("longitude", [None, 13.405], ids=str) -@pytest.mark.parametrize("country_code", [None, "DE"], ids=str) -def test_location_initialization( - latitude: float | None, - longitude: float | None, - country_code: str | None, -) -> None: - """Test location initialization with different combinations of parameters.""" - location = Location( - latitude=latitude, longitude=longitude, country_code=country_code - ) - assert location.latitude == latitude - assert location.longitude == longitude - assert location.country_code == country_code +def test_construction_valid() -> None: + """`Location(...)` with well-formed values stores each field verbatim.""" + location = Location(latitude=52.52, longitude=13.405, country_code="DE") + assert location.latitude == pytest.approx(52.52) + assert location.longitude == pytest.approx(13.405) + assert location.country_code == "DE" -@pytest.mark.parametrize( - "latitude, longitude, country_code, expected", - [ - (52.52, 13.405, "DE", "DE:(52.52, 13.40)"), - (None, None, "DE", "DE"), - (52.52, None, "DE", "DE:(52.52, ?)"), - (None, 13.405, "DE", "DE:(?, 13.40)"), - (52.52, 13.405, None, ":(52.52, 13.40)"), - (None, None, None, ""), - ], -) -def test_location_str( - latitude: float | None, - longitude: float | None, - country_code: str | None, - expected: str, -) -> None: - """Test the string representation of a Location.""" - location = Location( - latitude=latitude, longitude=longitude, country_code=country_code - ) - assert str(location) == expected +def test_construction_none_country_code() -> None: + """`Location(country_code=None)` succeeds.""" + location = Location(latitude=52.52, longitude=13.405, country_code=None) + assert location.country_code is None -@pytest.mark.parametrize( - "latitude, longitude", - [ - (-90.0, 0.0), - (90.0, 0.0), - (0.0, -180.0), - (0.0, 180.0), - (-90.0, -180.0), - (90.0, 180.0), - ], - ids=[ - "lat_min_boundary", - "lat_max_boundary", - "lon_min_boundary", - "lon_max_boundary", - "both_min_boundary", - "both_max_boundary", - ], -) -def test_location_boundary_values_accepted(latitude: float, longitude: float) -> None: - """Test that boundary latitude/longitude values are accepted.""" - location = Location(latitude=latitude, longitude=longitude, country_code=None) - assert location.latitude == latitude - assert location.longitude == longitude +def test_construction_wrapped_invalid_latitude() -> None: + """`Location(latitude=InvalidLatitude(...))` stores the wrapper.""" + invalid = InvalidLatitude(value=91.0) + location = Location(latitude=invalid, longitude=13.405, country_code="DE") + assert location.latitude == invalid + + +def test_construction_wrapped_invalid_longitude() -> None: + """`Location(longitude=InvalidLongitude(...))` stores the wrapper.""" + invalid = InvalidLongitude(value=181.0) + location = Location(latitude=52.52, longitude=invalid, country_code="DE") + assert location.longitude == invalid + + +def test_construction_wrapped_invalid_country_code() -> None: + """`Location(country_code=InvalidCountryCode(...))` stores the wrapper.""" + invalid = InvalidCountryCode(value="DEU") + location = Location(latitude=52.52, longitude=13.405, country_code=invalid) + assert location.country_code == invalid @pytest.mark.parametrize( - "latitude, longitude, match", - [ - (-90.001, 0.0, "latitude"), - (90.001, 0.0, "latitude"), - (0.0, -180.001, "longitude"), - (0.0, 180.001, "longitude"), - ], - ids=[ - "lat_below_min", - "lat_above_max", - "lon_below_min", - "lon_above_max", - ], + "latitude", + [-90.001, 90.001, math.nan, float("inf"), float("-inf")], + ids=["below_min", "above_max", "nan", "inf", "neg_inf"], ) -def test_location_out_of_range_raises( - latitude: float, longitude: float, match: str -) -> None: - """Test that out-of-range latitude/longitude raises ValueError.""" - with pytest.raises(ValueError, match=match): - Location(latitude=latitude, longitude=longitude, country_code=None) +def test_construction_rejects_plain_invalid_latitude(latitude: float) -> None: + """A plain `float` latitude outside `[-90, 90]` is rejected at construction.""" + with pytest.raises(ValueError, match=r"latitude .* is outside \[-90, 90\]"): + Location(latitude=latitude, longitude=13.405, country_code="DE") @pytest.mark.parametrize( - "latitude, longitude", - [ - (None, 13.405), - (52.52, None), - (None, None), - ], - ids=["lat_none", "lon_none", "both_none"], + "longitude", + [-180.001, 180.001, math.nan, float("inf"), float("-inf")], + ids=["below_min", "above_max", "nan", "inf", "neg_inf"], ) -def test_location_partial_none_accepted( - latitude: float | None, longitude: float | None -) -> None: - """Test that partial None coordinates are valid.""" - location = Location(latitude=latitude, longitude=longitude, country_code=None) - assert location.latitude == latitude - assert location.longitude == longitude +def test_construction_rejects_plain_invalid_longitude(longitude: float) -> None: + """A plain `float` longitude outside `[-180, 180]` is rejected at construction.""" + with pytest.raises(ValueError, match=r"longitude .* is outside \[-180, 180\]"): + Location(latitude=52.52, longitude=longitude, country_code="DE") @pytest.mark.parametrize( - "latitude, longitude, match", - [ - (math.nan, 0.0, "latitude"), - (0.0, math.nan, "longitude"), - ], - ids=["nan_lat", "nan_lon"], + "country_code", + ["", "D", "DEU", "DEUT"], + ids=["empty", "1_char", "3_chars", "4_chars"], ) -def test_location_nan_raises(latitude: float, longitude: float, match: str) -> None: - """Test that NaN latitude/longitude raises ValueError.""" - with pytest.raises(ValueError, match=match): - Location(latitude=latitude, longitude=longitude, country_code=None) +def test_construction_rejects_plain_invalid_country_code(country_code: str) -> None: + """A plain `str` country code not exactly 2 characters is rejected at construction.""" + with pytest.raises( + ValueError, match=r"country_code .* is not exactly 2 characters" + ): + Location(latitude=52.52, longitude=13.405, country_code=country_code) + + +def test_dataclasses_replace_valid() -> None: + """`dataclasses.replace` with a valid value returns an updated instance.""" + original = Location(latitude=52.52, longitude=13.405, country_code="DE") + replaced = dataclasses.replace(original, country_code="FR") + assert replaced.country_code == "FR" + assert replaced.latitude == pytest.approx(52.52) + assert replaced.longitude == pytest.approx(13.405) + + +def test_dataclasses_replace_enforces_invariant() -> None: + """`dataclasses.replace` with an out-of-invariant plain value is rejected.""" + original = Location(latitude=52.52, longitude=13.405, country_code="DE") + with pytest.raises(ValueError): + dataclasses.replace(original, country_code="DEU") diff --git a/tests/types/proto/v1alpha8/test_location.py b/tests/types/proto/v1alpha8/test_location.py index d8ddbe2f..80d36e05 100644 --- a/tests/types/proto/v1alpha8/test_location.py +++ b/tests/types/proto/v1alpha8/test_location.py @@ -8,11 +8,16 @@ import pytest from frequenz.api.common.v1alpha8.types import location_pb2 +from frequenz.client.common.types import ( + InvalidCountryCode, + InvalidLatitude, + InvalidLongitude, +) from frequenz.client.common.types.proto.v1alpha8 import location_from_proto @dataclass(frozen=True, kw_only=True) -class _ProtoConversionTestCase: # pylint: disable=too-many-instance-attributes +class _ProtoConversionTestCase: """Test case for protobuf conversion.""" name: str @@ -27,17 +32,19 @@ class _ProtoConversionTestCase: # pylint: disable=too-many-instance-attributes country_code: str """The country code to set in the protobuf message.""" - expected_none_latitude: bool = False - """The latitude is expected to be None.""" + expected_latitude: float | InvalidLatitude + """The expected `latitude` on the resulting `Location`.""" - expected_none_longitude: bool = False - """The longitude is expected to be None.""" + expected_longitude: float | InvalidLongitude + """The expected `longitude` on the resulting `Location`.""" - expected_none_country_code: bool = False - """The country code is expected to be None.""" + expected_country_code: str | InvalidCountryCode | None + """The expected `country_code` on the resulting `Location`. - expect_warning: bool = False - """Whether to expect a warning during conversion.""" + An empty `country_code` on the wire is normalized to `None`; a + non-empty `country_code` that is not exactly 2 characters is wrapped + in `InvalidCountryCode`. + """ @pytest.mark.parametrize( @@ -48,85 +55,93 @@ class _ProtoConversionTestCase: # pylint: disable=too-many-instance-attributes latitude=52.52, longitude=13.405, country_code="DE", + expected_latitude=52.52, + expected_longitude=13.405, + expected_country_code="DE", ), _ProtoConversionTestCase( name="boundary_latitude", latitude=90.0, longitude=13.405, country_code="DE", + expected_latitude=90.0, + expected_longitude=13.405, + expected_country_code="DE", ), _ProtoConversionTestCase( name="boundary_longitude", latitude=52.52, longitude=180.0, country_code="DE", + expected_latitude=52.52, + expected_longitude=180.0, + expected_country_code="DE", ), _ProtoConversionTestCase( name="invalid_latitude", latitude=91.0, longitude=13.405, country_code="DE", - expected_none_latitude=True, - expect_warning=True, + expected_latitude=InvalidLatitude(value=91.0), + expected_longitude=13.405, + expected_country_code="DE", ), _ProtoConversionTestCase( name="invalid_longitude", latitude=52.52, longitude=181.0, country_code="DE", - expected_none_longitude=True, - expect_warning=True, + expected_latitude=52.52, + expected_longitude=InvalidLongitude(value=181.0), + expected_country_code="DE", ), _ProtoConversionTestCase( name="empty_country_code", latitude=52.52, longitude=13.405, country_code="", - expected_none_country_code=True, - expect_warning=True, + expected_latitude=52.52, + expected_longitude=13.405, + expected_country_code=None, + ), + _ProtoConversionTestCase( + name="long_country_code", + latitude=52.52, + longitude=13.405, + country_code="DEU", + expected_latitude=52.52, + expected_longitude=13.405, + expected_country_code=InvalidCountryCode(value="DEU"), ), _ProtoConversionTestCase( name="all_invalid", latitude=-91.0, longitude=181.0, country_code="", - expected_none_latitude=True, - expected_none_longitude=True, - expected_none_country_code=True, - expect_warning=True, + expected_latitude=InvalidLatitude(value=-91.0), + expected_longitude=InvalidLongitude(value=181.0), + expected_country_code=None, ), ], ids=lambda case: case.name, ) -def test_from_proto( - caplog: pytest.LogCaptureFixture, case: _ProtoConversionTestCase -) -> None: - """Test conversion from protobuf message to Location.""" +def test_from_proto(case: _ProtoConversionTestCase) -> None: + """Wire values become plain values or Invalid* wrappers per invariant.""" proto = location_pb2.Location( latitude=case.latitude, longitude=case.longitude, country_code=case.country_code, ) - with caplog.at_level("WARNING"): - location = location_from_proto(proto) + location = location_from_proto(proto) - if case.expected_none_latitude: - assert location.latitude is None + if isinstance(case.expected_latitude, float): + assert isinstance(location.latitude, float) + assert location.latitude == pytest.approx(case.expected_latitude) else: - assert location.latitude == pytest.approx(case.latitude) - - if case.expected_none_longitude: - assert location.longitude is None - else: - assert location.longitude == pytest.approx(case.longitude) - - if case.expected_none_country_code: - assert location.country_code is None - else: - assert location.country_code == case.country_code - - if case.expect_warning: - assert len(caplog.records) > 0 - assert "Found issues in location:" in caplog.records[0].message + assert location.latitude == case.expected_latitude + if isinstance(case.expected_longitude, float): + assert isinstance(location.longitude, float) + assert location.longitude == pytest.approx(case.expected_longitude) else: - assert len(caplog.records) == 0 + assert location.longitude == case.expected_longitude + assert location.country_code == case.expected_country_code From 519637aedad4c3030b205f365cdb75001c3a600d Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 9 Jul 2026 12:39:28 +0200 Subject: [PATCH 4/5] Add safe accessors for `Location` attributes Add `get_latitude()`, `get_longitude()`, `get_country_code()` and `get_country_code_or_none()`. These accessors raise on invalid (or sometimes missing) data. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/_location.py | 114 +++++++++++- tests/microgrid/test_microgrid.py | 6 +- tests/types/_location/test_location.py | 176 ++++++++++++++++++ 3 files changed, 291 insertions(+), 5 deletions(-) diff --git a/src/frequenz/client/common/types/_location.py b/src/frequenz/client/common/types/_location.py index 0957166d..90e12d03 100644 --- a/src/frequenz/client/common/types/_location.py +++ b/src/frequenz/client/common/types/_location.py @@ -4,8 +4,9 @@ """Geographical co-ordinates of a place.""" from dataclasses import dataclass +from typing import assert_never -from .._exception import InvalidAttributeError +from .._exception import InvalidAttributeError, MissingFieldError class InvalidLatitudeError(InvalidAttributeError): @@ -185,7 +186,13 @@ class Location: [`InvalidLongitude`][..InvalidLongitude]; [`country_code`][.country_code] may be [`InvalidCountryCode`][..InvalidCountryCode] or `None` when the field - was unset on the wire. Users can pattern-match on the fields directly. + was unset on the wire. Users can pattern-match on the fields directly, + or call [`get_latitude()`][.get_latitude], + [`get_longitude()`][.get_longitude], + [`get_country_code()`][.get_country_code] and + [`get_country_code_or_none()`][.get_country_code_or_none] to obtain a + validated value or a clear + [`InvalidAttributeError`][...InvalidAttributeError] subclass. Constructing a `Location` with a plain `float` or `str` that violates its invariant raises `ValueError`; use the corresponding `Invalid*` @@ -198,6 +205,10 @@ class Location: A plain `float` when well-formed (in `[-90, 90]`); an [`InvalidLatitude`][...InvalidLatitude] wrapper when the wire delivered an out-of-range value. + + Tip: + Use [`Location.get_latitude()`][...Location.get_latitude] to obtain + a validated `float` or a clear error. """ longitude: float | InvalidLongitude @@ -206,6 +217,10 @@ class Location: A plain `float` when well-formed (in `[-180, 180]`); an [`InvalidLongitude`][...InvalidLongitude] wrapper when the wire delivered an out-of-range value. + + Tip: + Use [`Location.get_longitude()`][...Location.get_longitude] to obtain a + validated `float` or a clear error. """ country_code: str | InvalidCountryCode | None @@ -216,6 +231,10 @@ class Location: when the wire delivered a non-empty string of a different length; `None` when the field was unset on the wire (an empty string on the wire is normalized to `None` by the converter). + + Tip: + Use [`Location.get_country_code()`][...Location.get_country_code] to + obtain a validated `str` or a clear error. """ def __post_init__(self) -> None: @@ -253,6 +272,97 @@ def __post_init__(self) -> None: "invalid wire value" ) + def get_latitude(self) -> float: + """Return the latitude as a well-formed `float` in `[-90, 90]`. + + Returns: + The latitude, when it is a well-formed `float`. + + Raises: + InvalidLatitudeError: If [`latitude`][..latitude] is an + [`InvalidLatitude`][...InvalidLatitude]. The raw value is + available on the exception's `value` attribute. + """ + match self.latitude: + case InvalidLatitude(value=raw): + raise InvalidLatitudeError(self, "latitude", raw) + case float() | int() as valid: + return valid + case unknown: + assert_never(unknown) + + def get_longitude(self) -> float: + """Return the longitude as a well-formed `float` in `[-180, 180]`. + + Returns: + The longitude, when it is a well-formed `float`. + + Raises: + InvalidLongitudeError: If [`longitude`][..longitude] is an + [`InvalidLongitude`][...InvalidLongitude]. The raw value is + available on the exception's `value` attribute. + """ + match self.longitude: + case InvalidLongitude(value=raw): + raise InvalidLongitudeError(self, "longitude", raw) + case float() | int() as valid: + return valid + case unknown: + assert_never(unknown) + + def get_country_code(self) -> str: + """Return the country code as a well-formed 2-character `str`. + + Returns: + The country code, when it is a well-formed `str`. + + Raises: + MissingFieldError: If [`country_code`][..country_code] is + `None` (the field was not set on the wire). + InvalidCountryCodeError: If [`country_code`][..country_code] is + an [`InvalidCountryCode`][...InvalidCountryCode]. The raw + value is available on the exception's `value` attribute. + """ + match self.country_code: + case None: + raise MissingFieldError(self, "country_code") + case InvalidCountryCode(value=raw): + raise InvalidCountryCodeError(self, "country_code", raw) + case str() as valid: + return valid + case unknown: + assert_never(unknown) + + def get_country_code_or_none( + self, + ) -> str | None: + """Return the country code as a well-formed `str`, or `None` if unset. + + Same as [`get_country_code()`][..get_country_code] but returns + `None` instead of raising `MissingFieldError` when + [`country_code`][..country_code] is `None` (the field was not set + on the wire). Invalid country codes still raise + `InvalidCountryCodeError`. + + Returns: + The country code (when well-formed), or `None` (when the field + was not set on the wire). + + Raises: + InvalidCountryCodeError: If [`country_code`][..country_code] is + an [`InvalidCountryCode`][...InvalidCountryCode]. The raw + value is available on the exception's `value` attribute. + """ + match self.country_code: + case None: + return None + case InvalidCountryCode(value=raw): + raise InvalidCountryCodeError(self, "country_code", raw) + case str() as valid: + return valid + case unknown: + assert_never(unknown) + def __str__(self) -> str: """Return the short string representation of this instance.""" country = self.country_code or "" diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index 2bd22457..66b64a3f 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -46,9 +46,9 @@ def test_creation() -> None: assert info.delivery_area.code == "DE123" assert info.delivery_area.code_type == EnergyMarketCodeType.EUROPE_EIC assert info.location is not None - assert info.location.latitude == pytest.approx(52.52) - assert info.location.longitude == pytest.approx(13.405) - assert info.location.country_code == "DE" + assert info.location.get_latitude() == pytest.approx(52.52) + assert info.location.get_longitude() == pytest.approx(13.405) + assert info.location.get_country_code() == "DE" assert info.create_time == now assert info.is_active() is True diff --git a/tests/types/_location/test_location.py b/tests/types/_location/test_location.py index 13022d73..4908215b 100644 --- a/tests/types/_location/test_location.py +++ b/tests/types/_location/test_location.py @@ -8,10 +8,14 @@ import pytest +from frequenz.client.common._exception import MissingFieldError from frequenz.client.common.types import ( InvalidCountryCode, + InvalidCountryCodeError, InvalidLatitude, + InvalidLatitudeError, InvalidLongitude, + InvalidLongitudeError, Location, ) @@ -104,3 +108,175 @@ def test_dataclasses_replace_enforces_invariant() -> None: original = Location(latitude=52.52, longitude=13.405, country_code="DE") with pytest.raises(ValueError): dataclasses.replace(original, country_code="DEU") + + +# ============================================================ +# Accessors: get_latitude +# ============================================================ + + +@pytest.mark.parametrize( + "latitude", + [-90.0, 0.0, 90.0], + ids=["min_boundary", "middle", "max_boundary"], +) +def test_get_latitude_returns_valid(latitude: float) -> None: + """`get_latitude()` returns the stored `float` when well-formed.""" + location = Location(latitude=latitude, longitude=13.405, country_code="DE") + assert location.get_latitude() == pytest.approx(latitude) + + +def test_get_latitude_raises_for_wrapper() -> None: + """`get_latitude()` raises `InvalidLatitudeError` when latitude is wrapped.""" + location = Location( + latitude=InvalidLatitude(value=91.0), + longitude=13.405, + country_code="DE", + ) + with pytest.raises(InvalidLatitudeError) as exc_info: + location.get_latitude() + assert exc_info.value.attr_name == "latitude" + assert exc_info.value.value == pytest.approx(91.0) + + +# ============================================================ +# Accessors: get_longitude +# ============================================================ + + +@pytest.mark.parametrize( + "longitude", + [-180.0, 0.0, 180.0], + ids=["min_boundary", "middle", "max_boundary"], +) +def test_get_longitude_returns_valid(longitude: float) -> None: + """`get_longitude()` returns the stored `float` when well-formed.""" + location = Location(latitude=52.52, longitude=longitude, country_code="DE") + assert location.get_longitude() == pytest.approx(longitude) + + +def test_get_longitude_raises_for_wrapper() -> None: + """`get_longitude()` raises `InvalidLongitudeError` when longitude is wrapped.""" + location = Location( + latitude=52.52, + longitude=InvalidLongitude(value=181.0), + country_code="DE", + ) + with pytest.raises(InvalidLongitudeError) as exc_info: + location.get_longitude() + assert exc_info.value.attr_name == "longitude" + assert exc_info.value.value == pytest.approx(181.0) + + +# ============================================================ +# Accessors: get_country_code / get_country_code_or_none +# ============================================================ + + +def test_get_country_code_returns_valid() -> None: + """`get_country_code()` returns the stored `str` when well-formed.""" + location = Location(latitude=52.52, longitude=13.405, country_code="DE") + assert location.get_country_code() == "DE" + + +def test_get_country_code_raises_missing_for_none() -> None: + """`get_country_code()` raises `MissingFieldError` when country_code is `None`.""" + location = Location(latitude=52.52, longitude=13.405, country_code=None) + with pytest.raises(MissingFieldError) as exc_info: + location.get_country_code() + assert exc_info.value.attr_name == "country_code" + + +def test_get_country_code_raises_invalid_for_wrapper() -> None: + """`get_country_code()` raises `InvalidCountryCodeError` when wrapped.""" + location = Location( + latitude=52.52, + longitude=13.405, + country_code=InvalidCountryCode(value="DEU"), + ) + with pytest.raises(InvalidCountryCodeError) as exc_info: + location.get_country_code() + assert exc_info.value.attr_name == "country_code" + assert exc_info.value.value == "DEU" + + +def test_get_country_code_or_none_returns_valid() -> None: + """`get_country_code_or_none()` returns the stored `str` when well-formed.""" + location = Location(latitude=52.52, longitude=13.405, country_code="DE") + assert location.get_country_code_or_none() == "DE" + + +def test_get_country_code_or_none_returns_none_for_missing() -> None: + """`get_country_code_or_none()` returns `None` when country_code is `None`.""" + location = Location(latitude=52.52, longitude=13.405, country_code=None) + assert location.get_country_code_or_none() is None + + +def test_get_country_code_or_none_raises_for_wrapper() -> None: + """`get_country_code_or_none()` still raises `InvalidCountryCodeError` when wrapped.""" + location = Location( + latitude=52.52, + longitude=13.405, + country_code=InvalidCountryCode(value="DEU"), + ) + with pytest.raises(InvalidCountryCodeError) as exc_info: + location.get_country_code_or_none() + assert exc_info.value.attr_name == "country_code" + assert exc_info.value.value == "DEU" + + +# ============================================================ +# __str__ +# ============================================================ + + +@pytest.mark.parametrize( + "latitude, longitude, country_code, expected", + [ + (52.52, 13.405, "DE", "DE(52.52,13.40)"), + (52.52, 13.405, None, "(52.52,13.40)"), + ( + 52.52, + 13.405, + InvalidCountryCode(value="DEU"), + "(52.52,13.40)", + ), + ( + InvalidLatitude(value=91.0), + 13.405, + "DE", + "DE(,13.40)", + ), + ( + 52.52, + InvalidLongitude(value=181.0), + "DE", + "DE(52.52,)", + ), + ( + InvalidLatitude(value=91.0), + InvalidLongitude(value=181.0), + InvalidCountryCode(value="DEU"), + "(,)", + ), + ], + ids=[ + "valid", + "none_country", + "invalid_country", + "invalid_lat", + "invalid_lon", + "all_invalid", + ], +) +def test_str( + latitude: float | InvalidLatitude, + longitude: float | InvalidLongitude, + country_code: str | InvalidCountryCode | None, + expected: str, +) -> None: + """The string representation of a Location renders each field distinctly.""" + location = Location( + latitude=latitude, longitude=longitude, country_code=country_code + ) + assert str(location) == expected From 8dd41b432ce006688c883a744133f00d8ec4e148 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 8 Jul 2026 11:23:00 +0000 Subject: [PATCH 5/5] Add safe accessors for `Microgrid.location` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `Microgrid.get_location() -> Location`, the safe counterpart of the `Microgrid.location` field. It resolves the optional underlying field to a concrete `Location`: * `None` (the field was not set on the wire) → `MissingFieldError`. * `Location` → returned unchanged. Callers that need validated coordinates or a validated country code should chain through the new `Location.get_{latitude,longitude,country_code}()` accessors on the returned instance. Also generalize the file-local `_make_microgrid` test helper to accept both `delivery_area` and `location` as keyword-only arguments (both default to `None`), and update the existing `get_delivery_area()` tests to pass `delivery_area` by keyword — no behavior change, just the same helper covering both accessor test suites. Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/_microgrid.py | 21 ++++++++ tests/microgrid/test_microgrid.py | 53 +++++++++++++++++-- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index 8f558ca9..799c76fc 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -180,6 +180,27 @@ def get_delivery_area_or_none(self) -> DeliveryArea | None: case unknown: assert_never(unknown) + def get_location(self) -> Location: + """Return the location as a [`Location`][....types.Location]. + + This is the higher-level accessor for the [`location`][..location] + attribute: it resolves the field to a + [`Location`][....types.Location] or raises a clear, catchable error. + + The returned instance may still carry raw wire values that fail the + [`Location`][....types.Location] field invariants; use its own + `get_*()` accessors to obtain validated coordinates and country code. + + Returns: + The location, when it is set. + + Raises: + MissingFieldError: If the location is not set (`None`). + """ + if self.location is None: + raise MissingFieldError(self, "location") + return self.location + def __str__(self) -> str: """Return the ID of this microgrid as a string.""" name = f":{self.name}" if self.name else "" diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index 66b64a3f..7a56bbd0 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -20,7 +20,12 @@ InvalidDeliveryAreaError, ) from frequenz.client.common.microgrid import EnterpriseId, Microgrid, MicrogridId -from frequenz.client.common.types import Location +from frequenz.client.common.types import ( + InvalidCountryCode, + InvalidLatitude, + InvalidLongitude, + Location, +) def test_creation() -> None: @@ -191,15 +196,16 @@ def test_replace_preserves_construction() -> None: def _make_microgrid( - delivery_area: DeliveryArea | InvalidDeliveryArea | None, + delivery_area: DeliveryArea | InvalidDeliveryArea | None = None, + location: Location | None = None, ) -> Microgrid: - """Build a Microgrid with the given delivery area for accessor tests.""" + """Build a Microgrid with the given delivery area and location for accessor tests.""" return Microgrid( id=MicrogridId(1234), enterprise_id=EnterpriseId(5678), name="", delivery_area=delivery_area, - location=None, + location=location, create_time=datetime.now(timezone.utc), _active=True, _allow_construction=True, @@ -266,3 +272,42 @@ def test_get_delivery_area_or_none_error_is_value_error() -> None: info = _make_microgrid(InvalidDeliveryArea(code="", code_type=0)) with pytest.raises(ValueError): info.get_delivery_area_or_none() + + +def test_get_location_returns_location() -> None: + """`get_location()` returns the stored `Location` unchanged.""" + location = Location( + latitude=52.52, + longitude=13.405, + country_code="DE", + ) + info = _make_microgrid(location=location) + assert info.get_location() is location + + +def test_get_location_returns_lax_location() -> None: + """`get_location()` returns a `Location` even when it carries invalid wire values.""" + lax = Location( + latitude=InvalidLatitude(value=91.0), + longitude=InvalidLongitude(value=181.0), + country_code=InvalidCountryCode(value="DEU"), + ) + info = _make_microgrid(location=lax) + assert info.get_location() is lax + + +def test_get_location_raises_missing_for_none() -> None: + """`get_location()` raises `MissingFieldError` when the field is `None`.""" + info = _make_microgrid() + with pytest.raises( + MissingFieldError, + match=r"missing protobuf field 'location' in MID1234", + ): + info.get_location() + + +def test_get_location_error_is_value_error() -> None: + """The `MissingFieldError` raised by the accessor is also a `ValueError`.""" + info = _make_microgrid() + with pytest.raises(ValueError): + info.get_location()