Parse Quantity Strings (#824)

- Fix missing number in Quantity formatting for small values
- Add function to allow parsing of Quantity strings

RELEASE_NOTES.md

@@ -89,6 +89,8 @@
 
 - The `actor.ChannelRegistry` is now type-aware.
 
+- A new class method `Quantity.from_string()` has been added to allow the creation of `Quantity` objects from strings.
+
 ## Bug Fixes
 
 - 0W power requests are now not adjusted to exclusion bounds by the `PowerManager` and `PowerDistributor`, and are sent over to the microgrid API directly.
@@ -104,3 +106,5 @@
   A bug made the resampler interpret zero values as `None` when generating new samples, so if the result of the resampling is zero, the resampler would just produce `None` values.
 
 - The PowerManager no longer holds on to proposals from dead actors forever.  If an actor hasn't sent a new proposal in 60 seconds, the available proposal from that actor is dropped.
+
+- Fix `Quantity.__format__()` sometimes skipping the number for very small values.

pyproject.toml

@@ -99,6 +99,7 @@ dev-pytest = [
   "async-solipsism == 0.5",
   # For checking docstring code examples
   "frequenz-sdk[dev-examples]",
+  "hypothesis == 6.92.1",
 ]
 dev = [
   "frequenz-sdk[dev-mkdocs,dev-flake8,dev-formatting,dev-mkdocs,dev-mypy,dev-noxfile,dev-pylint,dev-pytest]",

src/frequenz/sdk/timeseries/_quantities.py

@@ -91,6 +91,42 @@ def zero(cls) -> Self:
         assert isinstance(_zero, cls)
         return _zero
 
+    @classmethod
+    def from_string(cls, string: str) -> Self:
+        """Return a quantity from a string representation.
+
+        Args:
+            string: The string representation of the quantity.
+
+        Returns:
+            A quantity object with the value given in the string.
+
+        Raises:
+            ValueError: If the string does not match the expected format.
+
+        """
+        split_string = string.split(" ")
+
+        if len(split_string) != 2:
+            raise ValueError(
+                f"Expected a string of the form 'value unit', got {string}"
+            )
+
+        assert cls._exponent_unit_map is not None
+        exp_map = cls._exponent_unit_map
+
+        for exponent, unit in exp_map.items():
+            if unit == split_string[1]:
+                instance = cls.__new__(cls)
+                try:
+                    instance._base_value = float(split_string[0]) * 10**exponent
+                except ValueError as error:
+                    raise ValueError(f"Failed to parse string '{string}'.") from error
+
+                return instance
+
+        raise ValueError(f"Unknown unit {split_string[1]}")
+
     @property
     def base_value(self) -> float:
         """Return the value of this quantity in the base unit.
@@ -163,6 +199,7 @@ def __str__(self) -> str:
         """
         return self.__format__("")
 
+    # pylint: disable=too-many-branches
     def __format__(self, __format_spec: str) -> str:
         """Return a formatted string representation of this quantity.
 
@@ -214,8 +251,22 @@ def __format__(self, __format_spec: str) -> str:
         if math.isinf(self._base_value) or math.isnan(self._base_value):
             return f"{self._base_value} {self._exponent_unit_map[0]}"
 
-        abs_value = abs(self._base_value)
-        exponent = math.floor(math.log10(abs_value)) if abs_value else 0
+        if abs_value := abs(self._base_value):
+            precision_pow = 10 ** (precision)
+            # Prevent numbers like 999.999999 being rendered as 1000 V
+            # instead of 1 kV.
+            # This could happen because the str formatting function does
+            # rounding as well.
+            # This is an imperfect solution that works for _most_ cases.
+            # isclose parameters were chosen according to the observed cases
+            if math.isclose(abs_value, precision_pow, abs_tol=1e-4, rel_tol=0.01):
+                # If the value is close to the precision, round it
+                exponent = math.ceil(math.log10(precision_pow))
+            else:
+                exponent = math.floor(math.log10(abs_value))
+        else:
+            exponent = 0
+
         unit_place = exponent - exponent % 3
         if unit_place < min(self._exponent_unit_map):
             unit = self._exponent_unit_map[min(self._exponent_unit_map.keys())]
@@ -225,11 +276,17 @@ def __format__(self, __format_spec: str) -> str:
             unit_place = max(self._exponent_unit_map)
         else:
             unit = self._exponent_unit_map[unit_place]
+
         value_str = f"{self._base_value / 10 ** unit_place:.{precision}f}"
-        stripped = value_str.rstrip("0").rstrip(".")
+
+        if value_str in ("-0", "0"):
+            stripped = value_str
+        else:
+            stripped = value_str.rstrip("0").rstrip(".")
+
         if not keep_trailing_zeros:
             value_str = stripped
-        unit_str = unit if stripped != "0" else self._exponent_unit_map[0]
+        unit_str = unit if stripped not in ("-0", "0") else self._exponent_unit_map[0]
         return f"{value_str} {unit_str}"
 
     def __add__(self, other: Self) -> Self:

tests/timeseries/test_quantities.py

@@ -5,7 +5,9 @@
 
 from datetime import timedelta
 
+import hypothesis
 import pytest
+from hypothesis import strategies as st
 
 from frequenz.sdk.timeseries._quantities import (
     Current,
@@ -105,8 +107,11 @@ def test_string_representation() -> None:
     assert (
         repr(Quantity(1.024445, exponent=0)) == "Quantity(value=1.024445, exponent=0)"
     )
+    assert f"{Quantity(0.50001, exponent=0):.0}" == "1"
     assert f"{Quantity(1.024445, exponent=0)}" == "1.024"
     assert f"{Quantity(1.024445, exponent=0):.0}" == "1"
+    assert f"{Quantity(0.124445, exponent=0):.0}" == "0"
+    assert f"{Quantity(0.50001, exponent=0):.0}" == "1"
     assert f"{Quantity(1.024445, exponent=0):.6}" == "1.024445"
 
     assert f"{Quantity(1.024445, exponent=3)}" == "1024.445"
@@ -128,7 +133,6 @@ def test_string_representation() -> None:
 
     assert f"{Fz1(1.024445, exponent=6)}" == "1024.445 kHz"
     assert f"{Fz2(1.024445, exponent=6)}" == "1.024 MHz"
-
     assert f"{Fz1(1.024445, exponent=9)}" == "1024445 kHz"
     assert f"{Fz2(1.024445, exponent=9)}" == "1.024 GHz"
 
@@ -147,6 +151,12 @@ def test_string_representation() -> None:
     assert f"{Fz1(-20)}" == "-20 Hz"
     assert f"{Fz1(-20000)}" == "-20 kHz"
 
+    assert f"{Power.from_watts(0.000124445):.0}" == "0 W"
+    assert f"{Energy.from_watt_hours(0.124445):.0}" == "0 Wh"
+    assert f"{Power.from_watts(-0.0):.0}" == "-0 W"
+    assert f"{Power.from_watts(0.0):.0}" == "0 W"
+    assert f"{Voltage.from_volts(999.9999850988388)}" == "1 kV"
+
 
 def test_isclose() -> None:
     """Test the isclose method of the quantities."""
@@ -355,7 +365,7 @@ def test_frequency() -> None:
     """Test the frequency class."""
     freq = Frequency.from_hertz(0.0000002)
     assert f"{freq:.9}" == "0.0000002 Hz"
-    freq = Frequency.from_kilohertz(600000.0)
+    freq = Frequency.from_kilohertz(600_000.0)
     assert f"{freq}" == "600 MHz"
 
     freq = Frequency.from_hertz(6.0)
@@ -527,3 +537,40 @@ def test_invalid_multiplications() -> None:
             _ = quantity * 200.0  # type: ignore
         with pytest.raises(TypeError):
             quantity *= 200.0  # type: ignore
+
+
+@pytest.mark.parametrize("quantity_type", [Power, Voltage, Current, Energy, Frequency])
+@pytest.mark.parametrize("exponent", [0, 3, 6, 9])
+@hypothesis.settings(
+    max_examples=1000
+)  # Set to have a decent amount of examples (default is 100)
+@hypothesis.seed(42)  # Seed that triggers a lot of problematic edge cases
+@hypothesis.given(value=st.floats(min_value=-1.0, max_value=1.0))
+def test_to_and_from_string(
+    quantity_type: type[Quantity], exponent: int, value: float
+) -> None:
+    """Test string parsing and formatting.
+
+    The parameters for this test are constructed to stay deterministic.
+
+    With a different (or random) seed or different max_examples the
+    test will show failing examples.
+
+    Fixing those cases was considered an unreasonable amount of work
+    at the time of writing.
+
+    For the future, one idea was to parse the string number after the first
+    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_str = f"{quantity:.{exponent}}"
+    from_string = quantity_type.from_string(quantity_str)
+    try:
+        assert f"{from_string:.{exponent}}" == quantity_str
+    except AssertionError as error:
+        pytest.fail(
+            f"Failed for {quantity.base_value} != from_string({from_string.base_value}) "
+            + f"with exponent {exponent} and source value '{value}': {error}"
+        )