FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Replace `Bounds` and `Lifetime` with `frequenz.core.math.Interval` by llucax · Pull Request #232 · frequenz-floss/frequenz-client-common-python · GitHub

Repository navigation

Replace Bounds and Lifetime with frequenz.core.math.Interval - #232

Closed
llucax wants to merge 8 commits into
frequenz-floss:v0.x.xfrom
llucax:interval
Closed

llucax wants to merge 8 commits into
frequenz-floss:v0.x.xfrom
llucax:interval

Conversation

llucax commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Wrapping Bounds and Lifetime is bringing protobuf low-level concepts into the high-level API.

These 2 messages provide an interval with a start and an end, exactly what frequenz.interval.Interval provides.

We replace both:

  • Bounds via a deprecation process, as it was already released.
  • Lifetime via a direct replacement, as it was not yet released.

The deprecated Bounds class now inherits from Interval, so it can be passed in places where an Interval is expected. New bounds_from_proto*2() functions are provided to convert from protobuf messages to Interval objects, and the old bounds_from_proto*() functions (without the 2 suffix) are deprecated.

The Lifetime type is completely removed, the lifetime_from_proto*() functions now return an Interval instead.

llucax requested a review from a team as a code owner July 2, 2026 13:06
llucax requested review from florian-wagner-frequenz and removed request for a team July 2, 2026 13:06
github-actions Bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) part:metrics Affects the metrics protobuf definitions part:microgrid Affects the microgrid protobuf definitions labels Jul 2, 2026

llucax commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

It will be a little simpler after frequenz-floss/frequenz-core-python#178 is merged and released.

llucax enabled auto-merge July 3, 2026 08:50
llucax added this to the v0.4.1 milestone Jul 3, 2026
llucax self-assigned this Jul 3, 2026
llucax added 8 commits July 3, 2026 11:12
Introduce `bounds_from_proto2` and `bounds_from_proto_with_issues2`
which return `frequenz.core.math.Interval[float | None]` instead of the
local `Bounds` type. `Bounds` is being retired in favor of the generic
`Interval` type from `frequenz-core`; these new symbols provide the
migration path for callers.

The existing `bounds_from_proto`/`_with_issues` remain unchanged and
will be deprecated in a follow-up commit.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Rewrite `Bounds` as a thin, deprecated subclass of
`frequenz.core.math.Interval[float | None]`, exposing the historical
`lower` / `upper` attribute names as deprecated `@property` aliases for
`Interval.start` / `Interval.end`.

`Bounds` now inherits Interval's validation, containment, string
conversion, and dataclass semantics; two overrides are added:

* `__eq__` — cross-compares against any `Interval` instance so
  `Bounds(lower=1, upper=2) == Interval(1, 2)` is `True`. Without this,
  dataclass-generated equality would reject mismatched types and break
  the migration UX.

* `__hash__` — restored (Python nullifies auto-hash whenever `__eq__`
  is overridden) and matches Interval's hashing of `(start, end)`.

Because `Bounds` now IS-A `Interval[float | None]`, code accepting
`Interval[float | None]` accepts `Bounds` instances. This unlocks
migrating field types on `MetricSample`, `ElectricalComponent`, and
`ElectricalComponentConnection` from `Bounds` to `Interval`
transparently.

Two visible behavioural changes on `Bounds`:

* Invalid-bounds `ValueError` message text moves from Bounds's old
  wording (`"Lower bound (X) must be less than or equal to upper bound
  (Y)"`) to Interval's (`"The start (X) can't be bigger than end (Y)"`).

* `str(Bounds(lower=None, upper=None))` now returns `"[∞, ∞]"` instead
  of `"[None, None]"` (`Interval`'s `__str__`).

We ignore the deprecation inside `bounds_from_proto()` to avoid a
double deprecation message because the function itself will be
deprecated in an upcoming commit.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Change the `MetricSample.bounds` field type from `list[Bounds]` to
`Sequence[Interval[float | None]]`. `Sequence` (covariant) allows
callers to keep passing `list[Bounds]` now that `Bounds` is a subclass
of `Interval[float | None]`; it also accepts pure
`list[Interval[float | None]]`, `tuple[Interval[float | None], ...]`,
etc..

This is a soft-breaking type-level change: downstream code holding a
variable typed `list[Bounds]` and passing it to `MetricSample` still
type-checks under Sequence covariance, and code reading `sample.bounds`
still supports iteration and indexing but no longer supports `.append()`
or other list-mutation methods. Runtime behaviour is unchanged (the
class is frozen — mutation was never legal). Allowing mutation of a
frozen `dataclass` was a bug anyways, it should have never been allowed.

The internal proto converter `_metric_bounds_from_proto` is rewritten in
place to build `list[Interval[float | None]]` directly via
`bounds_from_proto2`, so freshly-parsed samples no longer carry
deprecated `Bounds` instances. Existing `bounds_from_proto` remains
unchanged (deprecation lands in a follow-up commit).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Change the `metric_config_bounds` field on `ElectricalComponent` from
`Mapping[Metric | int, Bounds]` to `Mapping[Metric | int,
Interval[float | None]]`. `ElectricalComponent` is still unreleased, so
this is a direct type replacement, no need for backwards-compatibility.

The private converter `_metric_config_bounds_from_proto`, now calls
`bounds_from_proto2` internally so freshly-parsed components no longer
carry deprecated `Bounds` instances.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Change the `operational_lifetime` field on `ElectricalComponent` and
`ElectricalComponentConnection` from `Lifetime` to
`Interval[datetime | None]`. Both classes are still unreleased, so this
is a direct type replacement, no backwards-compatibility needed.

The default factory yields an unbounded
`Interval[datetime | None](None, None)` matching the historical "always
operational" semantics of `Lifetime()`.

`lifetime_from_proto` is rewritten to return `Interval[datetime | None]`
directly.

The invalid-lifetime error message text in the affected tests moves
from `"Start (X) must be before or equal to end (Y)"` (from
`Lifetime.__post_init__`) to `"The start (X) can't be bigger than end
(Y)"` (from `Interval.__post_init__`).

The `Lifetime` class itself will be removed in a follow-up commit.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Delete the `Lifetime` dataclass and its tests. `Lifetime` was added in
the current unreleased development cycle, and the previous commit
migrated every consumer to `frequenz.core.math.Interval`, so there is
no remaining user of the class.

The `lifetime_from_proto` conversion function stays — it still parses
`lifetime_pb2.Lifetime` protobuf messages, only its return type is now
`Interval[datetime | None]`.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Mark the two `Bounds`-returning proto converters `bounds_from_proto*`
and `bounds_from_proto_with_issues` as `@deprecated`, pointing callers
at `bounds_from_proto2` and `bounds_from_proto_with_issues2`
respectively. Both new variants return
`frequenz.core.math.Interval[float | None]` and are already used by
every internal call site.

`bounds_from_proto_with_issues` delegates to `bounds_from_proto`; the
internal call is wrapped in `warnings.catch_warnings()` so callers of
`_with_issues` see one `DeprecationWarning` (the outer one), not two.

The internal `Bounds(...)` construction inside `bounds_from_proto`
remains wrapped in `warnings.catch_warnings()` (added when `Bounds`
became `@deprecated`), so the two suppressions compose correctly and
each caller entry point emits exactly one warning identifying its own
replacement symbol.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

We had an internal discussion and @shsms was concerned about removing the domain-specific language from Bounds (lower/upper vs start/end). To move things forward I will close this one and keep things as they are, and add a BoundsSet in the future that is basically a copy of the IntervalSet proposed to core but specialized for bounds, and in the future we see if we abstract it in some way.

llucax closed this Jul 6, 2026
auto-merge was automatically disabled July 6, 2026 09:43

Pull request was closed

llucax added a commit to llucax/frequenz-client-common-python that referenced this pull request Aug 11, 2026
Like with `Lifetime`, `Location`, etc. we use the type system to encode
invalid data.

Replaces frequenz-floss#232 and part of frequenz-floss#239 and frequenz-floss#251.
llucax modified the milestones: v0.4.1, Dropped Aug 21, 2026
llucax deleted the interval branch September 16, 2026 11:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:metrics Affects the metrics protobuf definitions part:microgrid Affects the microgrid protobuf definitions part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL