Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
99 changes: 38 additions & 61 deletions src/frequenz/sdk/timeseries/_quantities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down
83 changes: 81 additions & 2 deletions tests/timeseries/test_quantities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit late to the review, but it might be better to call this _SANITIFY_NUM_CLASSES.. I just don't see a reason to skip this extra I...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just try to pronounce it with and without :P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree completely. When making typos, please make sure you don't skimp on the vowels. :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how I ended up typing this. But luckily we have smart completion, so typos are properly propagated and the code never fails!


_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
Copy link
Contributor

@Marenz Marenz Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# A very basic sanity check that are messing up the introspection

Someone def. messed up this comment :)

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 = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So close, we could have called these QTY_CTORS and pronounced them the "cutty cutters"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I made my first meme-PR!

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()
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "on the tests", I guess you mean just this test, because hypothesis also generates some int inputs when we give it a float range?

This function seems to just compare the value from quantity_type.from_string with the value from quantity._base_value. If they don't match up, maybe the from_string method just needs to convert to float as well?

And then they can be compared with quantity.is_close rather than a string comparison?

quantity_str = f"{quantity:.{exponent}}"
from_string = quantity_type.from_string(quantity_str)
try:
Expand Down