From df910b70a940f15e200b227216687fbe6f5b2ea7 Mon Sep 17 00:00:00 2001 From: "simon.voelcker" Date: Thu, 19 Feb 2026 10:33:19 +0100 Subject: [PATCH 1/5] Switch to conventional token span start/end semantics The tokens produced by our formula parser (lexer) contained spans with unusual and inconvenient start/end indices. The represented interval should be closed at the start and open at the end: [start,end). This makes it easy to slice the original formula and to compute the length of the span. Part of https://github.com/frequenz-floss/frequenz-sdk-python/issues/1356 Signed-off-by: simon.voelcker --- .../sdk/timeseries/formulas/_lexer.py | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/frequenz/sdk/timeseries/formulas/_lexer.py b/src/frequenz/sdk/timeseries/formulas/_lexer.py index 95d952e27..091365019 100644 --- a/src/frequenz/sdk/timeseries/formulas/_lexer.py +++ b/src/frequenz/sdk/timeseries/formulas/_lexer.py @@ -75,52 +75,49 @@ def __next__(self) -> _token.Token: # pylint: disable=too-many-branches comp_id = self._read_integer() if not comp_id: raise ValueError(f"Expected integer after '#' at position {pos}") - end_pos = pos + len(comp_id) + end_pos = pos + len(comp_id) + 1 # account for '#' return _token.Component( - span=( - pos + 1, - end_pos + 1, # account for '#' - ), + span=(pos, end_pos), id=comp_id, value=self._formula[pos:end_pos], ) if char == "+": _, char = next(self._iter) # consume operator - return _token.Plus(span=(pos + 1, pos + 1), value=char) + return _token.Plus(span=(pos, pos + 1), value=char) if char == "-": _, char = next(self._iter) - return _token.Minus(span=(pos + 1, pos + 1), value=char) + return _token.Minus(span=(pos, pos + 1), value=char) if char == "*": _, char = next(self._iter) - return _token.Mul(span=(pos + 1, pos + 1), value=char) + return _token.Mul(span=(pos, pos + 1), value=char) if char == "/": _, char = next(self._iter) - return _token.Div(span=(pos + 1, pos + 1), value=char) + return _token.Div(span=(pos, pos + 1), value=char) if char == "(": _, char = next(self._iter) - return _token.OpenParen(span=(pos + 1, pos + 1), value=char) + return _token.OpenParen(span=(pos, pos + 1), value=char) if char == ")": _, char = next(self._iter) - return _token.CloseParen(span=(pos + 1, pos + 1), value=char) + return _token.CloseParen(span=(pos, pos + 1), value=char) if char == ",": _, char = next(self._iter) - return _token.Comma(span=(pos + 1, pos + 1), value=char) + return _token.Comma(span=(pos, pos + 1), value=char) if char.isdigit(): num = self._read_number() end_pos = pos + len(num) - return _token.Number(span=(pos + 1, end_pos), value=num) + return _token.Number(span=(pos, end_pos), value=num) if char.isalpha(): symbol = self._read_symbol() end_pos = pos + len(symbol) - return _token.Symbol(span=(pos + 1, end_pos), value=symbol) + return _token.Symbol(span=(pos, end_pos), value=symbol) raise ValueError(f"Unexpected character '{char}' at position {pos}") From 345fe82da97127bc5a76cd39eccac02e7b54618d Mon Sep 17 00:00:00 2001 From: "simon.voelcker" Date: Fri, 20 Feb 2026 14:19:01 +0100 Subject: [PATCH 2/5] Improve formula validation - Added FormulaError and FormulaSyntaxError classes, using them in lexer and parser. - Changes to how functions are parsed: Detect unknown function first, then parse parameter list. - Removed mentions of terms, factors, unaries from error messages - Everything's an expression. - Better message for matching paranthesis. - Avoiding use of ASTNode.span in error messages, since it will be removed. Part of https://github.com/frequenz-floss/frequenz-sdk-python/issues/1356 Signed-off-by: simon.voelcker --- .../sdk/timeseries/formulas/_exceptions.py | 53 ++++++++++++ .../sdk/timeseries/formulas/_functions.py | 21 ++--- .../sdk/timeseries/formulas/_lexer.py | 13 ++- .../sdk/timeseries/formulas/_parser.py | 83 +++++++++++++++---- 4 files changed, 137 insertions(+), 33 deletions(-) create mode 100644 src/frequenz/sdk/timeseries/formulas/_exceptions.py diff --git a/src/frequenz/sdk/timeseries/formulas/_exceptions.py b/src/frequenz/sdk/timeseries/formulas/_exceptions.py new file mode 100644 index 000000000..d3012f90a --- /dev/null +++ b/src/frequenz/sdk/timeseries/formulas/_exceptions.py @@ -0,0 +1,53 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Exception classes for formulas parser and evaluator.""" + + +class FormulaError(Exception): + """Base class for exceptions in this module.""" + + +class FormulaSyntaxError(FormulaError): + """Exception raised for syntax errors in the formula.""" + + formula: str + span: tuple[int, int] + message: str + + def __init__( + self, *, formula: str, span: tuple[int, int] | None, message: str + ) -> None: + """Initialize this instance.""" + self._formula = formula + self._span = span + self._message = message + + def __str__(self) -> str: + """Return a string representation of the error. + + The span (if present) will be used to highlight the error location. + For long formulas, the beginning or end will be truncated as needed. + """ + if self._span is None: + return self._message + + formula_line = self._formula + span_padding = " " * self._span[0] + span_highlight = "^" * (self._span[1] - self._span[0]) + error_line = f"{span_padding}{span_highlight} {self._message}" + + char_limit: int = 80 + + if len(error_line) > char_limit: + # Remove characters from the left to fit error line within char limit + num_chars_to_truncate = len(error_line) - char_limit + error_line = error_line[num_chars_to_truncate:] + # Shift formula line by the same amount and insert leading ellipsis + formula_line = f"... {formula_line[num_chars_to_truncate+4:]}" + + if len(formula_line) > char_limit: + # Truncate the formula line, insert ellipsis + formula_line = f"{formula_line[:char_limit-4]} ..." + + return f"{formula_line}\n{error_line}" diff --git a/src/frequenz/sdk/timeseries/formulas/_functions.py b/src/frequenz/sdk/timeseries/formulas/_functions.py index 5397a52d7..0530fe750 100644 --- a/src/frequenz/sdk/timeseries/formulas/_functions.py +++ b/src/frequenz/sdk/timeseries/formulas/_functions.py @@ -85,19 +85,14 @@ async def unsubscribe(self) -> None: ) @classmethod - def from_string( - cls, name: str, params: list[AstNode[QuantityT]] - ) -> Function[QuantityT]: - """Create a function instance from its name.""" - match name.upper(): - case "COALESCE": - return Coalesce(params) - case "MAX": - return Max(params) - case "MIN": - return Min(params) - case _: - raise ValueError(f"Unknown function name: {name}") + def function_class_by_name(cls, name: str) -> type[Function[QuantityT]] | None: + """Return the function class corresponding to the given name.""" + known_functions: dict[str, type[Function[QuantityT]]] = { + "COALESCE": Coalesce, + "MAX": Max, + "MIN": Min, + } + return known_functions.get(name.upper()) @dataclass diff --git a/src/frequenz/sdk/timeseries/formulas/_lexer.py b/src/frequenz/sdk/timeseries/formulas/_lexer.py index 091365019..c6dfae17d 100644 --- a/src/frequenz/sdk/timeseries/formulas/_lexer.py +++ b/src/frequenz/sdk/timeseries/formulas/_lexer.py @@ -11,6 +11,7 @@ from typing_extensions import override from . import _token +from ._exceptions import FormulaSyntaxError from ._peekable import Peekable @@ -74,7 +75,11 @@ def __next__(self) -> _token.Token: # pylint: disable=too-many-branches _ = next(self._iter) # consume '#' comp_id = self._read_integer() if not comp_id: - raise ValueError(f"Expected integer after '#' at position {pos}") + raise FormulaSyntaxError( + formula=self._formula, + span=(pos + 1, pos + 2), + message="Expected integer", + ) end_pos = pos + len(comp_id) + 1 # account for '#' return _token.Component( span=(pos, end_pos), @@ -120,4 +125,8 @@ def __next__(self) -> _token.Token: # pylint: disable=too-many-branches end_pos = pos + len(symbol) return _token.Symbol(span=(pos, end_pos), value=symbol) - raise ValueError(f"Unexpected character '{char}' at position {pos}") + raise FormulaSyntaxError( + formula=self._formula, + span=(pos, pos + 1), + message="Unexpected character", + ) diff --git a/src/frequenz/sdk/timeseries/formulas/_parser.py b/src/frequenz/sdk/timeseries/formulas/_parser.py index 8b8ead2aa..968f59dbe 100644 --- a/src/frequenz/sdk/timeseries/formulas/_parser.py +++ b/src/frequenz/sdk/timeseries/formulas/_parser.py @@ -18,6 +18,7 @@ from . import _ast, _token from ._base_ast_node import AstNode +from ._exceptions import FormulaSyntaxError from ._formula import Formula from ._functions import FunCall, Function from ._lexer import Lexer @@ -65,6 +66,7 @@ def __init__( ): """Initialize the parser.""" self._name: str = name + self._formula: str = formula self._lexer: Peekable[_token.Token] = Peekable(Lexer(formula)) self._telemetry_fetcher: ResampledStreamFetcher = telemetry_fetcher self._create_method: Callable[[float], QuantityT] = create_method @@ -80,8 +82,10 @@ def _parse_term(self) -> AstNode[QuantityT] | None: next_factor = self._parse_factor() if next_factor is None: - raise ValueError( - f"Expected factor after operator at span: {token.span}" + raise FormulaSyntaxError( + formula=self._formula, + span=(token.span[1], token.span[1] + 1), + message="Expected expression", ) if isinstance(token, _token.Plus): @@ -104,7 +108,11 @@ def _parse_factor(self) -> AstNode[QuantityT] | None: token = next(self._lexer) next_unary = self._parse_unary() if next_unary is None: - raise ValueError(f"Expected unary after operator at span: {token.span}") + raise FormulaSyntaxError( + formula=self._formula, + span=(token.span[1], token.span[1] + 1), + message="Expected expression", + ) if isinstance(token, _token.Mul): unary = _ast.Mul(span=token.span, left=unary, right=next_unary) @@ -121,8 +129,10 @@ def _parse_unary(self) -> AstNode[QuantityT] | None: token = next(self._lexer) primary: AstNode[QuantityT] | None = self._parse_primary() if primary is None: - raise ValueError( - f"Expected primary expression after unary '-' at position {token.span}" + raise FormulaSyntaxError( + formula=self._formula, + span=(token.span[1], token.span[1] + 1), + message="Expected expression", ) zero_const = _ast.Constant(span=token.span, value=self._create_method(0.0)) @@ -136,11 +146,19 @@ def _parse_bracketed(self) -> AstNode[QuantityT] | None: expr: AstNode[QuantityT] | None = self._parse_term() if expr is None: - raise ValueError(f"Expected expression after '(' at position {oparen.span}") + raise FormulaSyntaxError( + formula=self._formula, + span=(oparen.span[1], oparen.span[1] + 1), + message="Expected expression", + ) token: _token.Token | None = self._lexer.peek() if token is None or not isinstance(token, _token.CloseParen): - raise ValueError(f"Expected ')' after expression at position {expr.span}") + raise FormulaSyntaxError( + formula=self._formula, + span=oparen.span, + message="Unmatched parenthesis", + ) _ = next(self._lexer) # consume ')' @@ -148,37 +166,62 @@ def _parse_bracketed(self) -> AstNode[QuantityT] | None: def _parse_function_call(self) -> AstNode[QuantityT] | None: fn_name: _token.Token = next(self._lexer) + function_class: type[Function[QuantityT]] | None = ( + Function.function_class_by_name(fn_name.value) + ) + + if function_class is None: + raise FormulaSyntaxError( + formula=self._formula, + span=fn_name.span, + message="Unknown function name", + ) + params: list[AstNode[QuantityT]] = [] token: _token.Token | None = self._lexer.peek() if token is None or not isinstance(token, _token.OpenParen): - raise ValueError( - f"Expected '(' after function name at position {fn_name.span}" + raise FormulaSyntaxError( + formula=self._formula, + span=(fn_name.span[1], fn_name.span[1] + 1), + message="Expected '('", ) + oparen = next(self._lexer) # consume '(' - _ = next(self._lexer) # consume '(' while True: param = self._parse_term() if param is None: - raise ValueError( - f"Expected argument in function call at position {fn_name.span}" + raise FormulaSyntaxError( + formula=self._formula, + span=(token.span[1], token.span[1] + 1), + message="Expected argument", ) params.append(param) token = self._lexer.peek() - if token is not None and isinstance(token, _token.Comma): + if token is None: + raise FormulaSyntaxError( + formula=self._formula, + span=oparen.span, + message="Unmatched parenthesis", + ) + + if isinstance(token, _token.Comma): _ = next(self._lexer) # consume ',' continue - if token is not None and isinstance(token, _token.CloseParen): + if isinstance(token, _token.CloseParen): _ = next(self._lexer) # consume ')' break - raise ValueError( - f"Expected ',' or ')' in function call at position {fn_name.span}" + + raise FormulaSyntaxError( + formula=self._formula, + span=token.span, + message="Expected ',' or ')'", ) return FunCall( span=fn_name.span, - function=Function.from_string(fn_name.value, params), + function=function_class(params), ) def _parse_primary(self) -> AstNode[QuantityT] | None: @@ -220,7 +263,11 @@ def make_component_stream_fetcher( def parse(self) -> Formula[QuantityT]: expr = self._parse_term() if expr is None: - raise ValueError("Empty formula.") + raise FormulaSyntaxError( + formula=self._formula, + span=None, + message="Empty formula", + ) return Formula( name=self._name, root=expr, From b1a2e78d15b625fe7c9e1f7779c9328e08fd1357 Mon Sep 17 00:00:00 2001 From: "simon.voelcker" Date: Fri, 20 Feb 2026 14:32:09 +0100 Subject: [PATCH 3/5] Add tests for formula validation Signed-off-by: simon.voelcker --- tests/timeseries/_formulas/test_formulas.py | 158 ++++++++++++++++++++ tests/timeseries/_formulas/test_lexer.py | 89 ++++++++--- 2 files changed, 228 insertions(+), 19 deletions(-) diff --git a/tests/timeseries/_formulas/test_formulas.py b/tests/timeseries/_formulas/test_formulas.py index 713c47594..0f0023b9e 100644 --- a/tests/timeseries/_formulas/test_formulas.py +++ b/tests/timeseries/_formulas/test_formulas.py @@ -17,6 +17,7 @@ from frequenz.quantities import Quantity from frequenz.sdk.timeseries import Sample +from frequenz.sdk.timeseries.formulas._exceptions import FormulaSyntaxError from frequenz.sdk.timeseries.formulas._formula import Formula, FormulaBuilder from frequenz.sdk.timeseries.formulas._parser import parse from frequenz.sdk.timeseries.formulas._resampled_stream_fetcher import ( @@ -172,6 +173,16 @@ async def test_simple(self) -> None: ([15.0, 3.0, 2.0], 2.5), ], ) + # Test unary minus: Parser inserts a zero before it. + await self.run_test( + "-#2", + "[f](0.0 - #2)", + [2], + [ + ([30.0], -30.0), + ([15.0], -15.0), + ], + ) async def test_compound(self) -> None: """Test compound formulas.""" @@ -333,6 +344,153 @@ async def test_max_min_coalesce(self) -> None: ) +class TestFormulaValidation: + """Tests for Formula validation.""" + + @pytest.mark.parametrize( + ("formula_str", "parsed_formula_str"), + [ + ("#1", "[f](#1)"), + ("-(1+#1)", "[f](0.0 - (1.0 + #1))"), + ("1*(2+3)", "[f](1.0 * (2.0 + 3.0))"), + ], + ) + async def test_parser_validation( + self, + formula_str: str, + parsed_formula_str: str, + ) -> None: + """Test formula parser validation.""" + try: + formula = parse( + name="f", + formula=formula_str, + create_method=Quantity, + telemetry_fetcher=MagicMock(spec=ResampledStreamFetcher), + ) + assert str(formula) == parsed_formula_str + except FormulaSyntaxError: + assert False, "Parser should not raise an error for this formula" + + @pytest.mark.parametrize( + ("formula_str", "expected_error_line"), + [ + ( + "1++", + " ^ Expected expression", + ), + ( + "1**", + " ^ Expected expression", + ), + ( + "--1", + " ^ Expected expression", + ), + ( + "(", + " ^ Expected expression", + ), + ( + "(1", + "^ Unmatched parenthesis", + ), + ( + "max", + " ^ Expected '('", + ), + ( + "max()", + " ^ Expected argument", + ), + ( + "max(1(", + " ^ Expected ',' or ')'", + ), + ( + "max(1", + " ^ Unmatched parenthesis", + ), + ( + "foo", + "^^^ Unknown function name", + ), + ( + "foo(1)", + "^^^ Unknown function name", + ), + ( + "max(1,,2)", + " ^ Expected argument", + ), + ], + ) + async def test_parser_validation_errors( + self, formula_str: str, expected_error_line: str + ) -> None: + """Test formula parser validation.""" + with pytest.raises(FormulaSyntaxError) as error: + _ = parse( + name="f", + formula=formula_str, + create_method=Quantity, + telemetry_fetcher=MagicMock(spec=ResampledStreamFetcher), + ) + + assert str(error.value) == f"{formula_str}\n{expected_error_line}" + + @pytest.mark.parametrize( + ("formula_str", "expected_error"), + [ + # Long formula with error near start -> Ellipsize end + ( + "max(coalesce(#1001, %1002, 0), coalesce(#1003, #1004, 0), coalesce(#1005, #1006, 0), coalesce(#1007, #1008, 0))", # noqa: E501 + "max(coalesce(#1001, %1002, 0), coalesce(#1003, #1004, 0), coalesce(#1005, #1 ...\n" + " ^ Unexpected character", + ), + # Long formula with error near the end -> Ellipsize start + ( + "max(coalesce(#1001, #1002, 0), coalesce(#1003, #1004, 0), coalesce(#1005, #1006, 0), coalesce(#10.07, #1008, 0))", # noqa: E501 + "... 03, #1004, 0), coalesce(#1005, #1006, 0), coalesce(#10.07, #1008, 0))\n" + " ^ Unexpected character", + ), + # Very long formula with error in the middle -> Ellipsize both sides + ( + "max(coalesce(#1001, #1002, 0), coalesce(#1003, #1004, 0), coalesce(#1005, #1006, 0), coalesce(#1007, #1008, 0)) :) " # noqa: E501 + "min(coalesce(#2001, #2002, 0), coalesce(#2003, #2004, 0), coalesce(#2005, #2006, 0), coalesce(#2007, #2008, 0))", # noqa: E501 + "... coalesce(#1005, #1006, 0), coalesce(#1007, #1008, 0)) :) min(coalesce(#2 ...\n" + " ^ Unexpected character", + ), + ], + ) + async def test_parser_validation_errors_in_long_formulas( + self, formula_str: str, expected_error: str + ) -> None: + """Test formula parser validation for long formulas.""" + with pytest.raises(FormulaSyntaxError) as error: + _ = parse( + name="f", + formula=formula_str, + create_method=Quantity, + telemetry_fetcher=MagicMock(spec=ResampledStreamFetcher), + ) + + assert str(error.value) == expected_error + assert all(len(line) <= 80 for line in str(error.value).splitlines()) + + async def test_empty_formula(self) -> None: + """Test formula parser validation.""" + with pytest.raises(FormulaSyntaxError) as error: + _ = parse( + name="f", + formula="", + create_method=Quantity, + telemetry_fetcher=MagicMock(spec=ResampledStreamFetcher), + ) + + assert str(error.value) == "Empty formula" + + class TestFormulaComposition: """Tests for formula channels.""" diff --git a/tests/timeseries/_formulas/test_lexer.py b/tests/timeseries/_formulas/test_lexer.py index 5d07f265d..2e287d83c 100644 --- a/tests/timeseries/_formulas/test_lexer.py +++ b/tests/timeseries/_formulas/test_lexer.py @@ -2,67 +2,118 @@ # Copyright © 2025 Frequenz Energy-as-a-Service GmbH """Tests for the formula lexer.""" +import pytest from frequenz.sdk.timeseries.formulas import _token +from frequenz.sdk.timeseries.formulas._exceptions import FormulaSyntaxError from frequenz.sdk.timeseries.formulas._lexer import Lexer def test_lexer() -> None: - """Test the Lexer reading integer tokens.""" + """Test the Lexer reading different kinds of tokens.""" formula = "#123 + coalesce(#1 / 10.0, 0.0)" lexer = Lexer(formula) component = next(lexer) assert isinstance(component, _token.Component) assert component.id == "123" - assert component.span == (1, 4) + assert component.value == "#123" + assert component.span == (0, 4) plus_op = next(lexer) assert isinstance(plus_op, _token.Plus) assert plus_op.value == "+" - assert plus_op.span == (6, 6) + assert plus_op.span == (5, 6) - number = next(lexer) - assert isinstance(number, _token.Symbol) - assert number.value == "coalesce" - assert number.span == (8, 15) + symbol = next(lexer) + assert isinstance(symbol, _token.Symbol) + assert symbol.value == "coalesce" + assert symbol.span == (7, 15) open_paren = next(lexer) assert isinstance(open_paren, _token.OpenParen) assert open_paren.value == "(" - assert open_paren.span == (16, 16) + assert open_paren.span == (15, 16) component = next(lexer) assert isinstance(component, _token.Component) assert component.id == "1" - assert component.span == (17, 18) + assert component.value == "#1" + assert component.span == (16, 18) div_op = next(lexer) assert isinstance(div_op, _token.Div) assert div_op.value == "/" - assert div_op.span == (20, 20) + assert div_op.span == (19, 20) number = next(lexer) assert isinstance(number, _token.Number) assert number.value == "10.0" - assert number.span == (22, 25) + assert number.span == (21, 25) comma = next(lexer) assert isinstance(comma, _token.Comma) assert comma.value == "," - assert comma.span == (26, 26) + assert comma.span == (25, 26) number = next(lexer) assert isinstance(number, _token.Number) assert number.value == "0.0" - assert number.span == (28, 30) + assert number.span == (27, 30) close_paren = next(lexer) assert isinstance(close_paren, _token.CloseParen) assert close_paren.value == ")" - assert close_paren.span == (31, 31) + assert close_paren.span == (30, 31) + + with pytest.raises(StopIteration): + next(lexer) + + +@pytest.mark.parametrize("whitespace", ["", " ", "\n", "\t"]) +def test_lexer_formula_is_only_whitespace(whitespace: str) -> None: + """Test handling of whitespace-only formula in the Lexer.""" + lexer = Lexer(whitespace) + tokens = list(lexer) + assert len(tokens) == 0 + + +def test_lexer_invalid_component_id() -> None: + """Test handling of invalid component IDs in the Lexer.""" + lexer = Lexer("#") + with pytest.raises(FormulaSyntaxError) as ex_info: + next(lexer) + error_lines = str(ex_info.value).splitlines() + assert error_lines == [ + "#", + " ^ Expected integer", + ] + + lexer = Lexer("#abc") + with pytest.raises(FormulaSyntaxError) as ex_info: + next(lexer) + error_lines = str(ex_info.value).splitlines() + assert error_lines == [ + "#abc", + " ^ Expected integer", + ] + + lexer = Lexer("#-1") + with pytest.raises(FormulaSyntaxError) as ex_info: + next(lexer) + error_lines = str(ex_info.value).splitlines() + assert error_lines == [ + "#-1", + " ^ Expected integer", + ] + - try: - _ = next(lexer) - assert False, "Expected StopIteration" - except StopIteration: - pass +def test_lexer_unexpected_character() -> None: + """Test handling of unexpected characters in the Lexer.""" + lexer = Lexer("123 $ 456") + with pytest.raises(FormulaSyntaxError) as ex_info: + list(lexer) + error_lines = str(ex_info.value).splitlines() + assert error_lines == [ + "123 $ 456", + " ^ Unexpected character", + ] From bade27e566e85a704088cc9784167fe8a33c60db Mon Sep 17 00:00:00 2001 From: "simon.voelcker" Date: Fri, 20 Feb 2026 14:32:24 +0100 Subject: [PATCH 4/5] Update release notes Signed-off-by: simon.voelcker --- RELEASE_NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 61ee6f2ad..d339d0def 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -14,4 +14,4 @@ ## Bug Fixes - +- Improved formula validation: Consistent error messages for invalid formulas and conventional span semantics. From 5a80e5e8ebab70dd2e8e3eed6b2e640aaf200d1a Mon Sep 17 00:00:00 2001 From: "simon.voelcker" Date: Fri, 20 Feb 2026 15:05:31 +0100 Subject: [PATCH 5/5] Remove unused ASTNode.span Signed-off-by: simon.voelcker --- .../sdk/timeseries/formulas/_base_ast_node.py | 3 --- .../sdk/timeseries/formulas/_parser.py | 22 +++++++------------ 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/frequenz/sdk/timeseries/formulas/_base_ast_node.py b/src/frequenz/sdk/timeseries/formulas/_base_ast_node.py index 56954139f..ee902b188 100644 --- a/src/frequenz/sdk/timeseries/formulas/_base_ast_node.py +++ b/src/frequenz/sdk/timeseries/formulas/_base_ast_node.py @@ -24,9 +24,6 @@ class AstNode(abc.ABC, Generic[QuantityT]): """An abstract syntax tree node representing a formula expression.""" - span: tuple[int, int] | None = None - """The span (start, end) of the expression in the input string.""" - @abc.abstractmethod async def evaluate(self) -> Sample[QuantityT] | QuantityT | None: """Evaluate the expression and return its numerical value.""" diff --git a/src/frequenz/sdk/timeseries/formulas/_parser.py b/src/frequenz/sdk/timeseries/formulas/_parser.py index 968f59dbe..be6eae863 100644 --- a/src/frequenz/sdk/timeseries/formulas/_parser.py +++ b/src/frequenz/sdk/timeseries/formulas/_parser.py @@ -89,9 +89,9 @@ def _parse_term(self) -> AstNode[QuantityT] | None: ) if isinstance(token, _token.Plus): - factor = _ast.Add(span=token.span, left=factor, right=next_factor) + factor = _ast.Add(left=factor, right=next_factor) elif isinstance(token, _token.Minus): - factor = _ast.Sub(span=token.span, left=factor, right=next_factor) + factor = _ast.Sub(left=factor, right=next_factor) token = self._lexer.peek() @@ -115,9 +115,9 @@ def _parse_factor(self) -> AstNode[QuantityT] | None: ) if isinstance(token, _token.Mul): - unary = _ast.Mul(span=token.span, left=unary, right=next_unary) + unary = _ast.Mul(left=unary, right=next_unary) elif isinstance(token, _token.Div): - unary = _ast.Div(span=token.span, left=unary, right=next_unary) + unary = _ast.Div(left=unary, right=next_unary) token = self._lexer.peek() @@ -135,8 +135,8 @@ def _parse_unary(self) -> AstNode[QuantityT] | None: message="Expected expression", ) - zero_const = _ast.Constant(span=token.span, value=self._create_method(0.0)) - return _ast.Sub(span=token.span, left=zero_const, right=primary) + zero_const = _ast.Constant(value=self._create_method(0.0)) + return _ast.Sub(left=zero_const, right=primary) return self._parse_primary() @@ -219,10 +219,7 @@ def _parse_function_call(self) -> AstNode[QuantityT] | None: message="Expected ',' or ')'", ) - return FunCall( - span=fn_name.span, - function=function_class(params), - ) + return FunCall(function=function_class(params)) def _parse_primary(self) -> AstNode[QuantityT] | None: token: _token.Token | None = self._lexer.peek() @@ -237,7 +234,6 @@ def make_component_stream_fetcher( if isinstance(token, _token.Component): _ = next(self._lexer) # consume token comp = _ast.TelemetryStream( - span=token.span, source=f"#{token.id}", metric_fetcher=make_component_stream_fetcher( self._telemetry_fetcher, ComponentId(int(token.id)) @@ -248,9 +244,7 @@ def make_component_stream_fetcher( if isinstance(token, _token.Number): _ = next(self._lexer) - return _ast.Constant( - span=token.span, value=self._create_method(float(token.value)) - ) + return _ast.Constant(value=self._create_method(float(token.value))) if isinstance(token, _token.OpenParen): return self._parse_bracketed()