Improve `Location` validation and error handling by llucax · Pull Request #248 · frequenz-floss/frequenz-client-common-python · GitHub

Make latitude and longitude required to match the protobuf definition while keeping country_code optional even when it is required because in practice it could be missing (encoded as "").

We now also allow storing invalid values, but encoded via new types: InvalidLatitude, InvalidLongitude, and InvalidCountryCode. This way users still can't read invalid values accidentally. This allows users to inspect invalid values if they need to, instead of only reporting via logging.

Add new safe accesors for Location attributes and Microgrid.location to allow users to safely access the values without having to check for validity themselves.

Part of #239 and #251.

Pull request overview

This PR updates frequenz.client.common’s Location model and its protobuf conversion to represent invalid wire values explicitly (via InvalidLatitude, InvalidLongitude, InvalidCountryCode) and to provide “safe” accessor methods that either return validated values or raise structured, catchable errors—aligning with the project’s goal of avoiding fragile _with_issues()-style string-based validation.

Changes:

Make Location.latitude/Location.longitude required (non-None) and represent out-of-range wire values with Invalid* wrapper types.
Update location_from_proto() to normalize/retain raw wire values via wrappers instead of logging warnings and dropping values.
Add semantic accessors (Location.get_*() and Microgrid.get_location()) that raise MissingFieldError / InvalidAttributeError subclasses for safe consumption.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/types/test_location.py	Removes the old Location tests that assumed optional coordinates and warning-based validation.
tests/types/proto/v1alpha8/test_location.py	Updates proto conversion tests to assert Invalid* wrappers instead of None + warnings.
tests/types/_location/init.py	Adds a dedicated test package for Location and its invalid-wrapper ecosystem.
tests/types/_location/test_location.py	New comprehensive tests for construction invariants, accessors, and __str__ rendering.
tests/types/_location/test_invalid_latitude.py	New tests for InvalidLatitude wrapper behavior.
tests/types/_location/test_invalid_longitude.py	New tests for InvalidLongitude wrapper behavior.
tests/types/_location/test_invalid_country_code.py	New tests for InvalidCountryCode wrapper behavior.
tests/types/_location/test_invalid_latitude_error.py	New tests for InvalidLatitudeError inheritance and message/value storage.
tests/types/_location/test_invalid_longitude_error.py	New tests for InvalidLongitudeError inheritance and message/value storage.
tests/types/_location/test_invalid_country_code_error.py	New tests for InvalidCountryCodeError inheritance and message/value storage.
tests/microgrid/test_microgrid.py	Updates microgrid tests to use the new safe accessors for Location and adds Microgrid.get_location() tests.
src/frequenz/client/common/types/proto/v1alpha8/_location.py	Changes proto→model conversion to wrap invalid values and normalize empty country_code to None.
src/frequenz/client/common/types/_location.py	Implements invalid-wrapper types, new accessor methods, and structured errors; updates invariants and string formatting.
src/frequenz/client/common/types/init.py	Re-exports new Invalid* wrapper types and their corresponding error types.
src/frequenz/client/common/microgrid/_microgrid.py	Adds Microgrid.get_location() accessor that raises MissingFieldError when absent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Same comment about emojis as here, otherwise LGTM

Updated.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

* Mention that `metric_config_bounds` keys also store the raw `0` for unspecified metrics (frequenz-floss#224). * Merge the two `Microgrid` bullets, which described the same addition. * Describe the `ElectricalComponent` boolean accessors (frequenz-floss#237), the `CategorySpecificInfo` field (frequenz-floss#257, frequenz-floss#273) and the aggregated `metric_config_bounds` (frequenz-floss#268). * Add a compact list of the smaller supporting types from frequenz-floss#248 and frequenz-floss#249 (`Invalid{Latitude,Longitude,CountryCode}`, `InvalidLifetime` and their errors). Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Fill the gaps found in a pre-release sanity check of `RELEASE_NOTES.md`: * Replace the template placeholder with a real release summary, including release stats and the note that this is a huge release despite the patch version, realizing the new library design while keeping backwards compatibility, with the breaking cleanup to follow in v0.5.0. * Document two breaking changes to symbols released in v0.4.0 that were missing from Upgrading: `Bounds` raising on `NaN` endpoints (frequenz-floss#254) and `MetricConnection.name` no longer being optional (frequenz-floss#257). * Complete the `UNSPECIFIED`-as-`0` bullet with `metric_config_bounds` keys (frequenz-floss#224), merge the duplicated `Microgrid` bullets, and briefly cover the `ElectricalComponent` accessors, `CategorySpecificInfo` and the smaller `Invalid*` types from frequenz-floss#237, frequenz-floss#248, frequenz-floss#249, frequenz-floss#257, frequenz-floss#268 and frequenz-floss#273. * Add the `str()` exception message change (frequenz-floss#258) to Bug Fixes. The `PaginationInfo` breaking change from frequenz-floss#217 is documented separately in frequenz-floss#285, which adds `InvalidPaginationInfo` and `pagination_info_from_proto2`.

llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jul 9, 2026

llucax self-assigned this Jul 9, 2026

llucax added this to the v0.4.1 milestone Jul 9, 2026

llucax force-pushed the err-location branch 4 times, most recently from 7a7ac21 to 1c9865c Compare July 13, 2026 08:20

llucax marked this pull request as ready for review July 13, 2026 08:21

llucax requested a review from a team as a code owner July 13, 2026 08:21

llucax requested review from ela-kotulska-frequenz and removed request for a team July 13, 2026 08:21

llucax enabled auto-merge July 13, 2026 08:21

llucax disabled auto-merge July 13, 2026 08:23

llucax enabled auto-merge July 13, 2026 10:55

llucax requested review from Marenz, Copilot and cwasicki July 13, 2026 10:55

Copilot started reviewing on behalf of llucax July 14, 2026 09:36

View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/frequenz/client/common/types/_location.py

Comment thread src/frequenz/client/common/types/_location.py

cwasicki reviewed Jul 14, 2026

View reviewed changes

llucax commented Jul 15, 2026

View reviewed changes

Comment thread src/frequenz/client/common/types/_location.py Outdated

llucax commented Jul 15, 2026

View reviewed changes

Comment thread src/frequenz/client/common/types/_location.py Outdated

llucax mentioned this pull request Jul 15, 2026

Standardize <invalid:VALUE> across __str__ #251

Closed

llucax disabled auto-merge July 16, 2026 09:58

llucax force-pushed the err-location branch from 1c9865c to 2e0d15c Compare July 16, 2026 10:08

llucax requested review from Copilot and cwasicki July 16, 2026 10:09

Copilot started reviewing on behalf of llucax July 16, 2026 10:09

View session

llucax enabled auto-merge July 16, 2026 10:10

llucax force-pushed the err-location branch 2 times, most recently from 57e7127 to a96f4a4 Compare July 16, 2026 10:11

Copilot AI reviewed Jul 16, 2026

View reviewed changes

Comment thread src/frequenz/client/common/types/_location.py

llucax added 5 commits July 16, 2026 12:15

Add Invalid{Latitude,Longitude,CountryCode classes

bde2127

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 <luca-frequenz@llucax.com>

Add safe accessors for Location attributes

519637a

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 <luca-frequenz@llucax.com>

llucax force-pushed the err-location branch from a96f4a4 to 8dd41b4 Compare July 16, 2026 10:16

cwasicki approved these changes Jul 16, 2026

View reviewed changes

llucax added this pull request to the merge queue Jul 16, 2026

Merged via the queue into frequenz-floss:v0.x.x with commit 4b009f4 Jul 16, 2026
9 checks passed

llucax deleted the err-location branch July 16, 2026 10:31

llucax linked an issue Aug 21, 2026 that may be closed by this pull request

Standardize <invalid:VALUE> across __str__ #251

Closed

llucax removed this from the v0.4.1 milestone Sep 15, 2026

This was referenced Sep 15, 2026

Consider using pycountry to validate location country codes #243

Open

Complete the release notes for v0.4.1 #286

Merged

Improve `Location` validation and error handling by llucax · Pull Request #248 · frequenz-floss/frequenz-client-common-python · GitHub

Repository navigation

Improve Location validation and error handling - #248

Improve Location validation and error handling#248
llucax merged 5 commits into
frequenz-floss:v0.x.xfrom
llucax:err-location

llucax commented Jul 9, 2026 •
edited
Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

cwasicki left a comment

Uh oh!

Uh oh!

Uh oh!

llucax commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Improve `Location` validation and error handling by llucax · Pull Request #248 · frequenz-floss/frequenz-client-common-python · GitHub

Repository navigation

Improve Location validation and error handling - #248

Improve Location validation and error handling#248llucax merged 5 commits intofrequenz-floss:v0.x.xfrequenz-floss/frequenz-client-common-python:v0.x.xfrom llucax:err-locationllucax/frequenz-client-common-python:err-locationCopy head branch name to clipboard

Conversation

llucax commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

cwasicki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llucax commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Improve Location validation and error handling#248
llucax merged 5 commits into
frequenz-floss:v0.x.xfrom
llucax:err-location

llucax commented Jul 9, 2026 •
edited
Loading