Skip to content

Commit b8908a0

Browse files
authored
Make all float annotations float | int (#256)
[PEP 484](https://peps.python.org/pep-0484/)'s [numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower) makes **`int` assignable to any `float`-annotated parameter or field**, while **at runtime `isinstance(1, float)` is `False`**. Any field annotated `float` can therefore silently store an `int`, and code unwrapping it with `match … case float():` falls through to `assert_never()`, calls to `float`-only methods like `hex()` crash, and `type()`-based dispatch misbehaves — despite everything type-checking cleanly. There is no clean fix in current Python (see the discussion in #250): runtime coercion at ingress was prototyped and measured ~2.3× slower on hot-path types, structural `Protocol` tricks don't close the widened variable and `Sequence` covariance holes, and narrowing match arms one by one leaves the annotation lying. So the decision is to stop lying instead: annotate every such value as `float | int`, which is what PEP 484 actually admits, at zero runtime cost. It is also forward-compatible with the typing-council proposal to make `float` mean `float | int` (python/typing-council#46). This commit replaces all `float` annotations with `FloatInt`, a type alias for `float | int`. Fixes #250.
2 parents 52c2934 + b9d7cbc commit b8908a0

16 files changed

Lines changed: 262 additions & 71 deletions

File tree

RELEASE_NOTES.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@
7070
7171
Malformed bounds are now preserved in the returned `MetricSample.bounds_set` as an `InvalidBoundsSet` (validity is encoded in the type), so the previous "bounds for ... is invalid, ignoring these bounds" major issue is no longer produced.
7272
73+
* `float`-typed fields and accessors are now annotated with the new `FloatInt` (`float | int`) type alias (see New Features), to be honest about what PEP 484's numeric tower actually admits. These symbols are affected:
74+
75+
* `frequenz.client.common.metrics.AggregatedMetricValue`: the `avg`, `min`, `max` and `raw` fields.
76+
* `frequenz.client.common.metrics.MetricSample`: the `value` field and the `as_single_value()` return type.
77+
* `frequenz.client.common.metrics.Bounds`: the `lower` and `upper` fields (shared with the new `BaseBounds` / `InvalidBounds` hierarchy).
78+
79+
Runtime behavior is completely unchanged: these fields could always end up storing `int` values (`x: float = 1` is legal even under `mypy --strict`), the annotations just didn't admit it. Reads that assign to `float`-typed destinations or do plain arithmetic keep type-checking as before. However, code that pattern-matches these values with a bare `case float():` arm — a latent runtime crash, since `isinstance(1, float)` is `False` — will now be flagged as non-exhaustive by strict type checkers and should be widened to `case float() | int():`, and calling `float`-only methods (e.g. `hex()`) on them now requires an explicit `float(...)` conversion.
80+
7381
## New Features
7482
7583
* Added 4 new electrical component classes for categories that previously collapsed into `UnrecognizedElectricalComponent`:
@@ -138,6 +146,11 @@
138146
139147
* Added a new `frequenz.client.common.microgrid.Microgrid` type with a raising `is_active()` method, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function.
140148
149+
* Added `frequenz.client.common.FloatInt`, a type alias for `float | int`.
150+
151+
PEP 484's numeric tower makes `int` assignable wherever `float` is annotated, even under `mypy --strict`, while at runtime `isinstance(1, float)` is `False` — so a plain `float` annotation silently admits values that crash `match … case float():` arms and `float`-only methods like `hex()`. The library now spells such annotations `FloatInt` instead of lying (see Upgrading); the alias docstring documents the trap in detail, including the inherent `bool ⊂ int` leak. New numeric fields (`Location` latitudes/longitudes, `PowerTransformer` voltages, bounds and bounds sets) use it as well. Values loaded from protobuf are unaffected in practice, as the wire always delivers real `float`s.
152+
141153
## Bug Fixes
142154
143155
* Fixed `EnumParityTest` so protobuf values whose Python member name exists with a different number fail parity checks instead of being treated as unmirrored protobuf values.
156+
* Fixed potential unexpected exceptions due to type-checking accepting `int` for code annotated to only accept `float`. Fixes #250.

src/frequenz/client/common/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010
UnrecognizedEnumValueError,
1111
UnspecifiedEnumValueError,
1212
)
13+
from ._float import FloatInt
1314

1415
__all__ = [
1516
"ClientCommonError",
17+
"FloatInt",
1618
"InvalidAttributeError",
1719
"MissingFieldError",
1820
"UnrecognizedEnumValueError",
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# License: MIT
2+
# Copyright © 2026 Frequenz Energy-as-a-Service GmbH
3+
4+
"""Honest type alias for floating-point values."""
5+
6+
from typing import TypeAlias
7+
8+
FloatInt: TypeAlias = float | int
9+
"""A `float` that may actually be an `int` at runtime.
10+
11+
[PEP 484's numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower)
12+
makes `int` assignable to any `float`-annotated parameter or field, so a plain
13+
`float` annotation is a lie: type checkers (even `mypy --strict`) happily
14+
accept `int` values, but `isinstance(1, float)` is `False` at runtime. That
15+
breaks `match … case float():` arms (an `int` value falls through to
16+
`assert_never()`), calls to `float`-only methods like `hex()`, and any other
17+
code dispatching on the concrete runtime type.
18+
19+
This library instead annotates such values as `FloatInt`, making the
20+
heterogeneity explicit: type checkers will push code reading these values to
21+
handle both branches, typically by matching with `case float() | int():`. See
22+
[issue #250](https://github.com/frequenz-floss/frequenz-client-common-python/issues/250)
23+
for the full analysis and the alternatives that were rejected.
24+
25+
Danger:
26+
`bool` is a subclass of `int`, so `True` and `False` also satisfy this
27+
alias. This is inherent to Python's type system and not guarded against.
28+
29+
Example:
30+
```python
31+
from typing import assert_never
32+
33+
from frequenz.client.common import FloatInt
34+
35+
36+
def describe(value: FloatInt | None) -> str:
37+
match value:
38+
case float() | int():
39+
return f"number {value}"
40+
case None:
41+
return "nothing"
42+
case unexpected:
43+
assert_never(unexpected)
44+
45+
46+
assert describe(1) == "number 1"
47+
assert describe(1.5) == "number 1.5"
48+
assert describe(None) == "nothing"
49+
```
50+
"""

src/frequenz/client/common/metrics/_bounds.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import Any, Self
1212

1313
from .._exception import InvalidAttributeError
14+
from .._float import FloatInt
1415

1516

1617
@dataclasses.dataclass(frozen=True, kw_only=True)
@@ -22,13 +23,13 @@ class BaseBounds:
2223
malformed wire data.
2324
"""
2425

25-
lower: float | int | None = None
26+
lower: FloatInt | None = None
2627
"""The lower bound.
2728
2829
If `None`, there is no lower bound.
2930
"""
3031

31-
upper: float | int | None = None
32+
upper: FloatInt | None = None
3233
"""The upper bound.
3334
3435
If `None`, there is no upper bound.
@@ -72,7 +73,7 @@ def __str__(self) -> str:
7273
"""Return a string representation of these bounds."""
7374
return f"[{self.lower},{self.upper}]"
7475

75-
def __contains__(self, item: float | None) -> bool:
76+
def __contains__(self, item: FloatInt | None) -> bool:
7677
"""Check whether a value is within these bounds.
7778
7879
The bounds are inclusive on both ends, and a `None` bound means these
@@ -173,7 +174,7 @@ def __init__(
173174
)
174175

175176

176-
def _end_covers_start(upper: float | None, lower: float | None) -> bool:
177+
def _end_covers_start(upper: FloatInt | None, lower: FloatInt | None) -> bool:
177178
"""Return whether an upper bound reaches a lower bound, treating `None` as ±∞.
178179
179180
Args:
@@ -190,7 +191,7 @@ def _end_covers_start(upper: float | None, lower: float | None) -> bool:
190191
return not upper < lower
191192

192193

193-
def _max_upper(first: float | None, second: float | None) -> float | None:
194+
def _max_upper(first: FloatInt | None, second: FloatInt | None) -> FloatInt | None:
194195
"""Return the larger of two upper bounds, where `None` means +∞.
195196
196197
Args:
@@ -226,7 +227,7 @@ def _sort_and_merge_bounds(bounds: Iterable[Bounds]) -> tuple[Bounds, ...]:
226227
return ()
227228

228229
with_none_lower: list[Bounds] = []
229-
with_real_lower: list[tuple[float, Bounds]] = []
230+
with_real_lower: list[tuple[FloatInt, Bounds]] = []
230231
for bound in all_bounds:
231232
if bound.lower is None:
232233
with_none_lower.append(bound)
@@ -306,7 +307,7 @@ def __post_init__(self) -> None:
306307
"""Normalize the bounds by sorting and merging overlapping ones."""
307308
object.__setattr__(self, "bounds", _sort_and_merge_bounds(self.bounds))
308309

309-
def __contains__(self, item: float | None) -> bool:
310+
def __contains__(self, item: FloatInt | None) -> bool:
310311
"""Check whether a value is within any bounds of this set.
311312
312313
Args:

src/frequenz/client/common/metrics/_sample.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from typing_extensions import deprecated
1414

1515
from .._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError
16+
from .._float import FloatInt
1617
from ._bounds import Bounds, BoundsSet, InvalidBoundsSet, InvalidBoundsSetError
1718
from ._metric import Metric
1819

@@ -45,16 +46,16 @@ class AggregatedMetricValue:
4546
are available.
4647
"""
4748

48-
avg: float
49+
avg: FloatInt
4950
"""The derived average value of the metric."""
5051

51-
min: float | None
52+
min: FloatInt | None
5253
"""The minimum measured value of the metric."""
5354

54-
max: float | None
55+
max: FloatInt | None
5556
"""The maximum measured value of the metric."""
5657

57-
raw: Sequence[float]
58+
raw: Sequence[FloatInt]
5859
"""All the raw individual values (it might be empty if not provided by the component)."""
5960

6061
def __str__(self) -> str:
@@ -193,7 +194,7 @@ class MetricSample:
193194
`MetricSample.get_metric()` to obtain a known member or a clear error.
194195
"""
195196

196-
value: float | AggregatedMetricValue | None
197+
value: FloatInt | AggregatedMetricValue | None
197198
"""The value of the sampled metric."""
198199

199200
bounds_set: BoundsSet | InvalidBoundsSet
@@ -244,7 +245,7 @@ def __init__(
244245
*,
245246
sample_time: datetime,
246247
metric: Metric | int,
247-
value: float | AggregatedMetricValue | None,
248+
value: FloatInt | AggregatedMetricValue | None,
248249
bounds_set: BoundsSet | InvalidBoundsSet | None = None,
249250
bounds: list[Bounds] | None = None,
250251
connection: MetricConnection | None = None,
@@ -307,10 +308,10 @@ def bounds(self) -> list[Bounds]:
307308

308309
def as_single_value(
309310
self, *, aggregation_method: AggregationMethod = AggregationMethod.AVG
310-
) -> float | None:
311+
) -> FloatInt | None:
311312
"""Return the value of this sample as a single value.
312313
313-
If [`value`][..value] is a `float`, it is returned as is. If `value`
314+
If [`value`][..value] is a number, it is returned as is. If `value`
314315
is an [`AggregatedMetricValue`][...AggregatedMetricValue], the value is
315316
aggregated using the provided `aggregation_method`.
316317

src/frequenz/client/common/microgrid/electrical_components/_power_transformer.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import dataclasses
77

8+
from ..._float import FloatInt
89
from ._electrical_component import ElectricalComponent
910

1011

@@ -22,13 +23,13 @@ class PowerTransformer(ElectricalComponent):
2223
than the input power.
2324
"""
2425

25-
primary_voltage: float
26+
primary_voltage: FloatInt
2627
"""The primary voltage of the transformer, in volts.
2728
2829
This is the input voltage that is stepped up or down.
2930
"""
3031

31-
secondary_voltage: float
32+
secondary_voltage: FloatInt
3233
"""The secondary voltage of the transformer, in volts.
3334
3435
This is the output voltage that is the result of stepping the primary

0 commit comments

Comments
 (0)