diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 096d36b6f..67b362f49 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -15,3 +15,4 @@ ## Bug Fixes - Fix grid current formula generator to add the operator `+` to the engine only when the component category is handled. +- Fix bug where sometimes the `base_value` of a `Quantity` could be of a different type than `float`. diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 307082223..53d0ee7e0 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -48,7 +48,22 @@ def __init__(self, value: float, exponent: int = 0) -> None: value: The value of this quantity in a given exponent of the base unit. exponent: The exponent of the base unit the given value is in. """ - self._base_value = value * 10**exponent + self._base_value = value * 10.0**exponent + + @classmethod + def _new(cls, value: float, *, exponent: int = 0) -> Self: + """Instantiate a new quantity subclass instance. + + Args: + value: The value of this quantity in a given exponent of the base unit. + exponent: The exponent of the base unit the given value is in. + + Returns: + A new quantity subclass instance. + """ + self = cls.__new__(cls) + self._base_value = value * 10.0**exponent + return self def __init_subclass__(cls, exponent_unit_map: dict[int, str]) -> None: """Initialize a new subclass of Quantity. @@ -78,15 +93,15 @@ def __init_subclass__(cls, exponent_unit_map: dict[int, str]) -> None: @classmethod def zero(cls) -> Self: - """Return a quantity with value 0. + """Return a quantity with value 0.0. Returns: - A quantity with value 0. + A quantity with value 0.0. """ _zero = cls._zero_cache.get(cls, None) if _zero is None: _zero = cls.__new__(cls) - _zero._base_value = 0 + _zero._base_value = 0.0 cls._zero_cache[cls] = _zero assert isinstance(_zero, cls) return _zero @@ -464,9 +479,7 @@ def from_celsius(cls, value: float) -> Self: Returns: A new temperature quantity. """ - power = cls.__new__(cls) - power._base_value = value - return power + return cls._new(value) def as_celsius(self) -> float: """Return the temperature in degrees Celsius. @@ -508,9 +521,7 @@ def from_watts(cls, watts: float) -> Self: Returns: A new power quantity. """ - power = cls.__new__(cls) - power._base_value = watts - return power + return cls._new(watts) @classmethod def from_milliwatts(cls, milliwatts: float) -> Self: @@ -522,9 +533,7 @@ def from_milliwatts(cls, milliwatts: float) -> Self: Returns: A new power quantity. """ - power = cls.__new__(cls) - power._base_value = milliwatts * 10**-3 - return power + return cls._new(milliwatts, exponent=-3) @classmethod def from_kilowatts(cls, kilowatts: float) -> Self: @@ -536,9 +545,7 @@ def from_kilowatts(cls, kilowatts: float) -> Self: Returns: A new power quantity. """ - power = cls.__new__(cls) - power._base_value = kilowatts * 10**3 - return power + return cls._new(kilowatts, exponent=3) @classmethod def from_megawatts(cls, megawatts: float) -> Self: @@ -550,9 +557,7 @@ def from_megawatts(cls, megawatts: float) -> Self: Returns: A new power quantity. """ - power = cls.__new__(cls) - power._base_value = megawatts * 10**6 - return power + return cls._new(megawatts, exponent=6) def as_watts(self) -> float: """Return the power in watts. @@ -690,9 +695,7 @@ def from_amperes(cls, amperes: float) -> Self: Returns: A new current quantity. """ - current = cls.__new__(cls) - current._base_value = amperes - return current + return cls._new(amperes) @classmethod def from_milliamperes(cls, milliamperes: float) -> Self: @@ -704,9 +707,7 @@ def from_milliamperes(cls, milliamperes: float) -> Self: Returns: A new current quantity. """ - current = cls.__new__(cls) - current._base_value = milliamperes * 10**-3 - return current + return cls._new(milliamperes, exponent=-3) def as_amperes(self) -> float: """Return the current in amperes. @@ -789,9 +790,7 @@ def from_volts(cls, volts: float) -> Self: Returns: A new voltage quantity. """ - voltage = cls.__new__(cls) - voltage._base_value = volts - return voltage + return cls._new(volts) @classmethod def from_millivolts(cls, millivolts: float) -> Self: @@ -803,9 +802,7 @@ def from_millivolts(cls, millivolts: float) -> Self: Returns: A new voltage quantity. """ - voltage = cls.__new__(cls) - voltage._base_value = millivolts * 10**-3 - return voltage + return cls._new(millivolts, exponent=-3) @classmethod def from_kilovolts(cls, kilovolts: float) -> Self: @@ -817,9 +814,7 @@ def from_kilovolts(cls, kilovolts: float) -> Self: Returns: A new voltage quantity. """ - voltage = cls.__new__(cls) - voltage._base_value = kilovolts * 10**3 - return voltage + return cls._new(kilovolts, exponent=3) def as_volts(self) -> float: """Return the voltage in volts. @@ -914,9 +909,7 @@ def from_watt_hours(cls, watt_hours: float) -> Self: Returns: A new energy quantity. """ - energy = cls.__new__(cls) - energy._base_value = watt_hours - return energy + return cls._new(watt_hours) @classmethod def from_kilowatt_hours(cls, kilowatt_hours: float) -> Self: @@ -928,9 +921,7 @@ def from_kilowatt_hours(cls, kilowatt_hours: float) -> Self: Returns: A new energy quantity. """ - energy = cls.__new__(cls) - energy._base_value = kilowatt_hours * 10**3 - return energy + return cls._new(kilowatt_hours, exponent=3) @classmethod def from_megawatt_hours(cls, megawatt_hours: float) -> Self: @@ -942,9 +933,7 @@ def from_megawatt_hours(cls, megawatt_hours: float) -> Self: Returns: A new energy quantity. """ - energy = cls.__new__(cls) - energy._base_value = megawatt_hours * 10**6 - return energy + return cls._new(megawatt_hours, exponent=6) def as_watt_hours(self) -> float: """Return the energy in watt hours. @@ -1039,9 +1028,7 @@ def from_hertz(cls, hertz: float) -> Self: Returns: A new frequency quantity. """ - frequency = cls.__new__(cls) - frequency._base_value = hertz - return frequency + return cls._new(hertz) @classmethod def from_kilohertz(cls, kilohertz: float) -> Self: @@ -1053,9 +1040,7 @@ def from_kilohertz(cls, kilohertz: float) -> Self: Returns: A new frequency quantity. """ - frequency = cls.__new__(cls) - frequency._base_value = kilohertz * 10**3 - return frequency + return cls._new(kilohertz, exponent=3) @classmethod def from_megahertz(cls, megahertz: float) -> Self: @@ -1067,9 +1052,7 @@ def from_megahertz(cls, megahertz: float) -> Self: Returns: A new frequency quantity. """ - frequency = cls.__new__(cls) - frequency._base_value = megahertz * 10**6 - return frequency + return cls._new(megahertz, exponent=6) @classmethod def from_gigahertz(cls, gigahertz: float) -> Self: @@ -1081,9 +1064,7 @@ def from_gigahertz(cls, gigahertz: float) -> Self: Returns: A new frequency quantity. """ - frequency = cls.__new__(cls) - frequency._base_value = gigahertz * 10**9 - return frequency + return cls._new(gigahertz, exponent=9) def as_hertz(self) -> float: """Return the frequency in hertz. @@ -1152,9 +1133,7 @@ def from_percent(cls, percent: float) -> Self: Returns: A new percentage quantity. """ - percentage = cls.__new__(cls) - percentage._base_value = percent - return percentage + return cls._new(percent) @classmethod def from_fraction(cls, fraction: float) -> Self: @@ -1166,9 +1145,7 @@ def from_fraction(cls, fraction: float) -> Self: Returns: A new percentage quantity. """ - percentage = cls.__new__(cls) - percentage._base_value = fraction * 100 - return percentage + return cls._new(fraction * 100) def as_percent(self) -> float: """Return this quantity as a percentage. diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index b4ae25c8e..4ea1d0510 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -3,12 +3,15 @@ """Tests for quantity types.""" +import inspect from datetime import timedelta +from typing import Callable import hypothesis import pytest from hypothesis import strategies as st +from frequenz.sdk.timeseries import _quantities from frequenz.sdk.timeseries._quantities import ( Current, Energy, @@ -45,6 +48,48 @@ class Fz2( """Frequency quantity with broad exponent unit map.""" +_CtorType = Callable[[float], Quantity] + +# This is the current number of subclasses. This probably will get outdated, but it will +# provide at least some safety against something going really wrong and end up testing +# an empty list. With this we should at least make sure we are not testing less classes +# than before. We don't get the actual number using len(_QUANTITY_SUBCLASSES) because it +# would defeat the purpose of the test. +_SANITFY_NUM_CLASSES = 7 + +_QUANTITY_SUBCLASSES = [ + cls + for _, cls in inspect.getmembers( + _quantities, + lambda m: inspect.isclass(m) and issubclass(m, Quantity) and m is not Quantity, + ) +] + +# A very basic sanity check that are messing up the introspection +assert len(_QUANTITY_SUBCLASSES) >= _SANITFY_NUM_CLASSES + +_QUANTITY_BASE_UNIT_STRINGS = [ + cls._new(0).base_unit # pylint: disable=protected-access + for cls in _QUANTITY_SUBCLASSES +] +for unit in _QUANTITY_BASE_UNIT_STRINGS: + assert unit is not None + +_QUANTITY_CTORS = [ + method + for cls in _QUANTITY_SUBCLASSES + for _, method in inspect.getmembers( + cls, + lambda m: inspect.ismethod(m) + and m.__name__.startswith("from_") + and m.__name__ != ("from_string"), + ) +] +# A very basic sanity check that are messing up the introspection. There are actually +# many more constructors than classes, but this still works as a very basic check. +assert len(_QUANTITY_CTORS) >= _SANITFY_NUM_CLASSES + + def test_zero() -> None: """Test the zero value for quantity.""" assert Quantity(0.0) == Quantity.zero() @@ -101,6 +146,31 @@ def test_zero() -> None: assert Percentage.zero() is Percentage.zero() # It is a "singleton" +@pytest.mark.parametrize("quantity_ctor", _QUANTITY_CTORS) +def test_base_value_from_ctor_is_float(quantity_ctor: _CtorType) -> None: + """Test that the base value always is a float.""" + quantity = quantity_ctor(1) + assert isinstance(quantity.base_value, float) + + +@pytest.mark.parametrize("quantity_type", _QUANTITY_SUBCLASSES + [Quantity]) +def test_base_value_from_zero_is_float(quantity_type: type[Quantity]) -> None: + """Test that the base value always is a float.""" + quantity = quantity_type.zero() + assert isinstance(quantity.base_value, float) + + +@pytest.mark.parametrize( + "quantity_type, unit", zip(_QUANTITY_SUBCLASSES, _QUANTITY_BASE_UNIT_STRINGS) +) +def test_base_value_from_string_is_float( + quantity_type: type[Quantity], unit: str +) -> None: + """Test that the base value always is a float.""" + quantity = quantity_type.from_string(f"1 {unit}") + assert isinstance(quantity.base_value, float) + + def test_string_representation() -> None: """Test the string representation of the quantities.""" assert str(Quantity(1.024445, exponent=0)) == "1.024" @@ -539,6 +609,9 @@ def test_invalid_multiplications() -> None: quantity *= 200.0 # type: ignore +# We can't use _QUANTITY_TYPES here, because it will break the tests, as hypothesis +# will generate more values, some of which are unsupported by the quantities. See the +# test comment for more details. @pytest.mark.parametrize("quantity_type", [Power, Voltage, Current, Energy, Frequency]) @pytest.mark.parametrize("exponent", [0, 3, 6, 9]) @hypothesis.settings( @@ -563,8 +636,14 @@ def test_to_and_from_string( generation and regenerate it with the more appropriate unit and precision. """ quantity = quantity_type.__new__(quantity_type) - # pylint: disable=protected-access - quantity._base_value = value * 10**exponent + quantity._base_value = value * 10**exponent # pylint: disable=protected-access + # The above should be replaced with: + # quantity = quantity_type._new( # pylint: disable=protected-access + # value, exponent=exponent + # ) + # But we can't do that now, because, you guessed it, it will also break the tests + # (_new() will use 10.0**exponent instead of 10**exponent, which seems to have some + # effect on the tests. quantity_str = f"{quantity:.{exponent}}" from_string = quantity_type.from_string(quantity_str) try: