Skip to content

Commit 7fc9844

Browse files
committed
Make sure Quantity.base_value is always a float
In Python `float` is a bit weird, because `isinstance(1, float)` is `False` but still `int` is a subtype of `float` in terms of the type system, so `int` will be accepted when a `float` is specified in arguments for example. This leads to quantities that can have `int` as base value if it was constructed with a `int` instead of a `float`, which is unintuitive when the `base_value` type hint is `float` and can potentially cause problems. Signed-off-by: Leandro Lucarella <[email protected]>
1 parent e4d7623 commit 7fc9844

File tree

2 files changed

+65
-5
lines changed

2 files changed

+65
-5
lines changed

src/frequenz/sdk/timeseries/_quantities.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def __init__(self, value: float, exponent: int = 0) -> None:
4848
value: The value of this quantity in a given exponent of the base unit.
4949
exponent: The exponent of the base unit the given value is in.
5050
"""
51-
self._base_value = value * 10**exponent
51+
self._base_value = value * 10.0**exponent
5252

5353
@classmethod
5454
def _new(cls, value: float, *, exponent: int = 0) -> Self:
@@ -62,7 +62,7 @@ def _new(cls, value: float, *, exponent: int = 0) -> Self:
6262
A new quantity subclass instance.
6363
"""
6464
self = cls.__new__(cls)
65-
self._base_value = value * 10**exponent
65+
self._base_value = value * 10.0**exponent
6666
return self
6767

6868
def __init_subclass__(cls, exponent_unit_map: dict[int, str]) -> None:
@@ -93,15 +93,15 @@ def __init_subclass__(cls, exponent_unit_map: dict[int, str]) -> None:
9393

9494
@classmethod
9595
def zero(cls) -> Self:
96-
"""Return a quantity with value 0.
96+
"""Return a quantity with value 0.0.
9797
9898
Returns:
99-
A quantity with value 0.
99+
A quantity with value 0.0.
100100
"""
101101
_zero = cls._zero_cache.get(cls, None)
102102
if _zero is None:
103103
_zero = cls.__new__(cls)
104-
_zero._base_value = 0
104+
_zero._base_value = 0.0
105105
cls._zero_cache[cls] = _zero
106106
assert isinstance(_zero, cls)
107107
return _zero

tests/timeseries/test_quantities.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33

44
"""Tests for quantity types."""
55

6+
import inspect
67
from datetime import timedelta
8+
from typing import Callable
79

810
import hypothesis
911
import pytest
1012
from hypothesis import strategies as st
1113

14+
from frequenz.sdk.timeseries import _quantities
1215
from frequenz.sdk.timeseries._quantities import (
1316
Current,
1417
Energy,
@@ -45,6 +48,38 @@ class Fz2(
4548
"""Frequency quantity with broad exponent unit map."""
4649

4750

51+
_CtorType = Callable[[float], Quantity]
52+
53+
_QUANTITY_SUBCLASSES = [
54+
cls
55+
for _, cls in inspect.getmembers(
56+
_quantities,
57+
lambda m: inspect.isclass(m) and issubclass(m, Quantity) and m is not Quantity,
58+
)
59+
]
60+
# Make a simple sanity check, 7 is the current number of subclasses
61+
assert len(_QUANTITY_SUBCLASSES) >= 7
62+
63+
_QUANTITY_BASE_UNIT_STRINGS = [
64+
cls._new(0).base_unit # pylint: disable=protected-access
65+
for cls in _QUANTITY_SUBCLASSES
66+
]
67+
for unit in _QUANTITY_BASE_UNIT_STRINGS:
68+
assert unit is not None
69+
70+
_QUANTITY_CTORS = [
71+
method
72+
for cls in _QUANTITY_SUBCLASSES
73+
for _, method in inspect.getmembers(
74+
cls,
75+
lambda m: inspect.ismethod(m)
76+
and m.__name__.startswith("from_")
77+
and m.__name__ != ("from_string"),
78+
)
79+
]
80+
assert len(_QUANTITY_CTORS) >= 7
81+
82+
4883
def test_zero() -> None:
4984
"""Test the zero value for quantity."""
5085
assert Quantity(0.0) == Quantity.zero()
@@ -101,6 +136,31 @@ def test_zero() -> None:
101136
assert Percentage.zero() is Percentage.zero() # It is a "singleton"
102137

103138

139+
@pytest.mark.parametrize("quantity_ctor", _QUANTITY_CTORS)
140+
def test_base_value_from_ctor_is_float(quantity_ctor: _CtorType) -> None:
141+
"""Test that the base value always is a float."""
142+
quantity = quantity_ctor(1)
143+
assert isinstance(quantity.base_value, float)
144+
145+
146+
@pytest.mark.parametrize("quantity_type", _QUANTITY_SUBCLASSES + [Quantity])
147+
def test_base_value_from_zero_is_float(quantity_type: type[Quantity]) -> None:
148+
"""Test that the base value always is a float."""
149+
quantity = quantity_type.zero()
150+
assert isinstance(quantity.base_value, float)
151+
152+
153+
@pytest.mark.parametrize(
154+
"quantity_type, unit", zip(_QUANTITY_SUBCLASSES, _QUANTITY_BASE_UNIT_STRINGS)
155+
)
156+
def test_base_value_from_string_is_float(
157+
quantity_type: type[Quantity], unit: str
158+
) -> None:
159+
"""Test that the base value always is a float."""
160+
quantity = quantity_type.from_string(f"1 {unit}")
161+
assert isinstance(quantity.base_value, float)
162+
163+
104164
def test_string_representation() -> None:
105165
"""Test the string representation of the quantities."""
106166
assert str(Quantity(1.024445, exponent=0)) == "1.024"

0 commit comments

Comments
 (0)