Rewrite FormulaFormatter.format to format_formula

Signed-off-by: Christian Parpart <[email protected]>

Author: christianparpart

RELEASE_NOTES.md

@@ -24,7 +24,7 @@
 
 - NaN values are treated as missing when gaps are determined in the `OrderedRingBuffer`.
 
-- A new class `FormulaFormatter` has been added to format formulas from `FormulaEngine`. For convenience, `FormulaEngine` now provides an `__str__` override that makes use of this to make formulas more intuitively introspectible.
+- Now when printing `FormulaEngine` for debugging purposes the the formula will be shown in infix notation, which should be easier to read.
 
 ## Bug Fixes
 

src/frequenz/sdk/timeseries/_formula_engine/__init__.py

@@ -4,13 +4,11 @@
 """A formula engine for applying formulas."""
 from ._formula_engine import FormulaEngine, FormulaEngine3Phase
 from ._formula_engine_pool import FormulaEnginePool
-from ._formula_formatter import FormulaFormatter
 from ._resampled_formula_builder import ResampledFormulaBuilder
 
 __all__ = [
     "FormulaEngine",
     "FormulaEngine3Phase",
     "FormulaEnginePool",
-    "FormulaFormatter",
     "ResampledFormulaBuilder",
 ]

src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py

@@ -20,7 +20,7 @@
 from .. import Sample, Sample3Phase
 from .._quantities import Quantity, QuantityT
 from ._formula_evaluator import FormulaEvaluator
-from ._formula_formatter import FormulaFormatter
+from ._formula_formatter import format_formula
 from ._formula_steps import (
     Adder,
     Averager,
@@ -318,7 +318,7 @@ def __str__(self) -> str:
             if len(self._builder._build_stack) > 0
             else self._builder._steps
         )
-        return FormulaFormatter.format(steps)
+        return format_formula(steps)
 
     def new_receiver(
         self, name: str | None = None, max_size: int = 50
@@ -642,7 +642,7 @@ def __str__(self) -> str:
             A string representation of the formula.
         """
         steps = self._steps if len(self._steps) > 0 else self._build_stack
-        return FormulaFormatter.format(steps)
+        return format_formula(steps)
 
     def build(self) -> FormulaEngine[QuantityT]:
         """Create a formula engine with the steps and fetchers that have been pushed.

src/frequenz/sdk/timeseries/_formula_engine/_formula_formatter.py

@@ -179,20 +179,7 @@ def __init__(self) -> None:
         """Initialize the FormulaFormatter."""
         self._stack = list[StackItem]()
 
-    @classmethod
-    def format(cls, postfix_expr: list[FormulaStep]) -> str:
-        """Return the formula as a string in infix notation.
-
-        Args:
-            postfix_expr: The steps of the formula in postfix notation order.
-
-        Returns:
-            str: The formula in infix notation.
-        """
-        formatter = FormulaFormatter()
-        return formatter._format(postfix_expr)
-
-    def _format(self, postfix_expr: list[FormulaStep]) -> str:
+    def format(self, postfix_expr: list[FormulaStep]) -> str:
         """Format the postfix expression to infix notation.
 
         Args:
@@ -216,7 +203,7 @@ def _format(self, postfix_expr: list[FormulaStep]) -> str:
                 case Averager():
                     value = (
                         # pylint: disable=protected-access
-                        f"avg({', '.join(self._format([f]) for f in step.fetchers)})"
+                        f"avg({', '.join(self.format([f]) for f in step.fetchers)})"
                     )
                     self._stack.append(StackItem(value, OperatorPrecedence.PRIMARY, 1))
                 case Clipper():
@@ -270,3 +257,16 @@ def _pop_two_from_stack(self) -> tuple[StackItem, StackItem]:
         right = self._stack.pop()
         left = self._stack.pop()
         return left, right
+
+
+def format_formula(postfix_expr: list[FormulaStep]) -> str:
+    """Return the formula as a string in infix notation.
+
+    Args:
+        postfix_expr: The steps of the formula in postfix notation order.
+
+    Returns:
+        str: The formula in infix notation.
+    """
+    formatter = FormulaFormatter()
+    return formatter.format(postfix_expr)

src/frequenz/sdk/timeseries/_formula_engine/_formula_steps.py

@@ -7,7 +7,7 @@
 
 import math
 from abc import ABC, abstractmethod
-from typing import Generic, Optional, Sequence
+from typing import Generic, Sequence
 
 from frequenz.channels import Receiver
 

tests/timeseries/test_formula_formatter.py

@@ -4,15 +4,13 @@
 """Tests for the FormulaFormatter."""
 
 
-import logging
-
 from frequenz.channels import Broadcast
 from pytest_mock import MockerFixture
 
 from frequenz.sdk import microgrid
 from frequenz.sdk.timeseries import Sample
 from frequenz.sdk.timeseries._formula_engine._formula_engine import FormulaBuilder
-from frequenz.sdk.timeseries._formula_engine._formula_formatter import FormulaFormatter
+from frequenz.sdk.timeseries._formula_engine._formula_formatter import format_formula
 from frequenz.sdk.timeseries._formula_engine._formula_steps import (
     Averager,
     Clipper,
@@ -25,6 +23,7 @@
 from frequenz.sdk.timeseries._quantities import Percentage, Quantity
 from tests.timeseries.mock_microgrid import MockMicrogrid
 
+
 def build_formula(formula: str) -> list[FormulaStep]:
     """Parse the formula and returns the steps.
 
@@ -63,7 +62,7 @@ def reconstruct(formula: str) -> str:
         The reconstructed formula in infix notation.
     """
     steps = build_formula(formula)
-    reconstructed = FormulaFormatter.format(steps)
+    reconstructed = format_formula(steps)
     if formula != reconstructed:
         print(f"Formula: input {formula} != output {reconstructed}")
     return reconstructed
@@ -106,16 +105,16 @@ def test_functions(self) -> None:
         # For simplicity, we only test with constant values.
         # fmt: off
         # flake8: noqa: E501
-        assert FormulaFormatter.format([ConstantValue(2), ConstantValue(3), Minimizer()]) == "min(2, 3)"
-        assert FormulaFormatter.format([ConstantValue(2), ConstantValue(3), Maximizer()]) == "max(2, 3)"
-        assert FormulaFormatter.format([ConstantValue(3.5), Clipper(0.0, 1.0)]) == "clip(0.0, 3.5, 1.0)"
+        assert format_formula([ConstantValue(2), ConstantValue(3), Minimizer()]) == "min(2, 3)"
+        assert format_formula([ConstantValue(2), ConstantValue(3), Maximizer()]) == "max(2, 3)"
+        assert format_formula([ConstantValue(3.5), Clipper(0.0, 1.0)]) == "clip(0.0, 3.5, 1.0)"
         # flake8: enable
         # fmt: on
 
     def test_function_avg(self) -> None:
         """Test that the avg function is formatted correctly."""
         # This tests the special case of the avg function with no arguments.
-        assert FormulaFormatter.format([Averager[Percentage]([])]) == "avg()"
+        assert format_formula([Averager[Percentage]([])]) == "avg()"
 
     async def test_higher_order_formula(self, mocker: MockerFixture) -> None:
         """Test that the formula is formatted correctly for a higher-order formula."""