From cf96d921268dbeac95a022ade50e960c78af375c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 22:38:53 +0000 Subject: [PATCH 1/3] chore(language): a where comparison is one grammar rule over the expression grammar's arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The where grammar's three comparison rules — a name against a literal, two relation columns, and position() against an integer — are one rule, side side, where a side is the expression grammar's ARITHMETIC. What a side is, resolution decides with the schema in hand, and it hands back the same typed nodes with the same messages. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EEeM2YoAk4Xr2uWB5qwMsH --- src/math_spec/_expression_parser.py | 15 ++- src/math_spec/_where_parser.py | 136 +++++++++++------------ src/math_spec/resolution.py | 161 +++++++++++++++++++++------- tests/test_parser.py | 105 ++++++++++++------ tests/test_validation.py | 45 ++++++++ tests/typesetting/test_golden.py | 12 ++- 6 files changed, 321 insertions(+), 153 deletions(-) diff --git a/src/math_spec/_expression_parser.py b/src/math_spec/_expression_parser.py index bb404e82..52bdf216 100644 --- a/src/math_spec/_expression_parser.py +++ b/src/math_spec/_expression_parser.py @@ -412,8 +412,12 @@ def with_children(node: ArithmeticNode, recurse: Callable[[ArithmeticNode], Arit # --------------------------------------------------------------------------- -def _build_grammar() -> pp.ParserElement: - """``inf`` is a ``pp.Keyword`` rather than a ``pp.Literal``, which would match the prefix of ``inflow``.""" +def _build_grammar() -> tuple[pp.ParserElement, pp.ParserElement]: + """The arithmetic grammar, and the expression grammar that puts one comparison over it. + + ``inf`` is a ``pp.Keyword`` rather than a ``pp.Literal``, which would + match the prefix of ``inflow``. + """ arith = pp.Forward() inf_literal = (pp.Keyword('.inf') | pp.Keyword('inf')).set_parse_action(lambda: NumberNode(float('inf'))) @@ -449,9 +453,10 @@ def _build_grammar() -> pp.ParserElement: arith <<= add_sub comparator = pp.one_of(list(get_args(ComparisonOperator))) - return (arith + pp.Optional(comparator + arith)).set_parse_action( + expression = (arith + pp.Optional(comparator + arith)).set_parse_action( lambda t: ComparisonNode(t[1], t[0], t[2]) if len(t) == 3 else t[0] ) + return arith, expression def _make_func_call(tokens: pp.ParseResults) -> FunctionCallNode: @@ -485,7 +490,9 @@ def _make_power(tokens: pp.ParseResults) -> ArithmeticNode: return items[0] if len(items) == 1 else BinaryOperatorNode('**', items[0], items[2]) -_GRAMMAR = _build_grammar() +#: The arithmetic half on its own, for the where grammar to put a predicate's +#: comparator over: one grammar for what a side may say, wherever it stands. +ARITHMETIC, _GRAMMAR = _build_grammar() #: How deep a tree the language admits. Every pass over an expression recurses, diff --git a/src/math_spec/_where_parser.py b/src/math_spec/_where_parser.py index 4a6cf2de..3f56aa65 100644 --- a/src/math_spec/_where_parser.py +++ b/src/math_spec/_where_parser.py @@ -4,7 +4,12 @@ """The where-string grammar and the ``Unresolved*`` nodes it emits, package-private. -The resolved vocabulary lives in :mod:`math_spec.program`. +A where string is a boolean algebra over comparisons, and a comparison's +sides are the expression grammar's own arithmetic. What a side *is* — a +parameter, a dimension, a relation column, a ``position()`` — only the schema +knows, so the grammar hands both sides over bare and +:mod:`math_spec.resolution` reads them. The resolved vocabulary lives in +:mod:`math_spec.program`. """ from __future__ import annotations @@ -15,12 +20,13 @@ import pyparsing as pp -from math_spec._expression_parser import NAME, REAL, parse_text +from math_spec._expression_parser import ARITHMETIC, NAME, children, parse_text from math_spec.program import AndNode, BooleanLiteralNode, NotNode, OrNode, PredicateOperator, where_children if TYPE_CHECKING: from collections.abc import Callable + from math_spec._expression_parser import ArithmeticNode from math_spec.program import WhereNode # --------------------------------------------------------------------------- @@ -36,109 +42,83 @@ class UnresolvedNameNode: @dataclass(frozen=True) -class UnresolvedComparisonNode: - """A comparison against an unresolved name. ``resolution.py`` types it.""" +class ColumnNode: + """``relation.column`` on a side of a comparison — the one place the language names a column.""" - name: str - op: PredicateOperator - value: float | str - #: Whether the right-hand side arrived in quotes. A bare word is ambiguous - #: — it may name a declaration — and resolution refuses it for that reason; - #: a quoted one is unambiguously a label, which is the only way to write - #: ``combined-cycle`` or a date. Consumed by resolution, never lowered. - quoted: bool = False + relation: str + column: str + @property + def shown(self) -> str: + """The column as the file wrote it, for an error message.""" + return f'{self.relation}.{self.column}' -@dataclass(frozen=True) -class UnresolvedPositionNode: - """``position(dim[, by=relation, within=columns]) i`` before the names are checked; ``resolution.py`` types it.""" - dimension: str - op: PredicateOperator - position: int - by: str | None = None - into: tuple[str, ...] | None = None +@dataclass(frozen=True) +class QuotedNode: + """A right-hand side that arrived in quotes. + A bare word is ambiguous — it may name a declaration — and resolution + refuses it for that reason; a quoted one is unambiguously a label, which + is the only way to write ``combined-cycle`` or a date. + """ -#: What resolution rewrites away on the where side — the three nodes whose -#: left-hand side is still a name the schema has not been asked about. -UnresolvedWhereNode = UnresolvedNameNode | UnresolvedComparisonNode | UnresolvedPositionNode + value: str -# --------------------------------------------------------------------------- -# Grammar -# --------------------------------------------------------------------------- - +@dataclass(frozen=True) +class UnresolvedComparisonNode: + """``side side`` before the sides are read; ``resolution.py`` decides what each is. -class _Quoted(str): - """A right-hand side that arrived in quotes; :func:`_comparison` turns it back into a flag.""" + A side is the expression grammar's arithmetic, so a name, a number and a + ``position(...)`` call all arrive as the nodes an expression would carry + them in; a relation column and a quoted label have nodes of their own. + """ - __slots__ = () + left: ArithmeticNode | ColumnNode + op: PredicateOperator + right: ArithmeticNode | ColumnNode | QuotedNode -def _position_comparison(tokens: pp.ParseResults) -> UnresolvedPositionNode: - """``position(dim[, by=relation, within=columns]) i`` off the tokens the grammar captured.""" - dimension, *call, op, at = tokens - by = str(call[0]) if call else None - into = tuple(str(token) for token in call[1]) if len(call) > 1 else None - return UnresolvedPositionNode(str(dimension), op, at, by, into) +#: What resolution rewrites away on the where side — the two nodes whose +#: leaves are still names the schema has not been asked about. +UnresolvedWhereNode = UnresolvedNameNode | UnresolvedComparisonNode -def _comparison(tokens: pp.ParseResults) -> UnresolvedComparisonNode: - """``name literal`` off the tokens the grammar captured, the quoted marker turned into a flag.""" - name, op, value = tokens - quoted = isinstance(value, _Quoted) - return UnresolvedComparisonNode(str(name), op, str(value) if quoted else value, quoted) +# --------------------------------------------------------------------------- +# Grammar +# --------------------------------------------------------------------------- def _build_where_grammar() -> pp.ParserElement: """Build the pyparsing grammar for where strings. Both quote characters are accepted because YAML already owns one of them. - ``NOT`` binds tightest, then ``AND``, then ``OR``. ``position(...)`` leads - the alternation, since ``position`` would otherwise be read as a bare name. + ``NOT`` binds tightest, then ``AND``, then ``OR``. A comparison is tried + before a bare name, since its left side begins with one. """ where_expr = pp.Forward() true_lit = pp.CaselessKeyword('True').set_parse_action(lambda: BooleanLiteralNode(True)) false_lit = pp.CaselessKeyword('False').set_parse_action(lambda: BooleanLiteralNode(False)) - # pyrefly: ignore[implicit-any-lambda] - number = pp.Regex(rf'-?({REAL}|\d+)').set_parse_action(lambda t: float(t[0])) - # pyrefly: ignore[implicit-any-lambda] - position = pp.Regex(r'-?\d+').set_parse_action(lambda t: int(t[0])) - name = pp.Regex(NAME) - + # pyrefly: ignore[implicit-any-lambda] + column = pp.Regex(rf'({NAME})\.({NAME})').set_parse_action(lambda t: ColumnNode(*t[0].split('.'))) quoted = (pp.QuotedString("'", esc_char='\\') | pp.QuotedString('"', esc_char='\\')).set_parse_action( - lambda t: _Quoted(t[0]) - ) - - column = pp.Regex(rf'{NAME}(\.{NAME})?') - columns = name | (pp.Suppress('[') + pp.DelimitedList(name) + pp.Suppress(']')) - grouped_within = pp.Group(pp.Suppress(',') + pp.Suppress(pp.Keyword('within')) + pp.Suppress('=') + columns) - grouped_by = ( - pp.Suppress(',') + pp.Suppress(pp.Keyword('by')) + pp.Suppress('=') + name + pp.Optional(grouped_within) + # pyrefly: ignore[implicit-any-lambda] + lambda t: QuotedNode(t[0]) ) comparator = pp.one_of(list(get_args(PredicateOperator))) - position_call = ( - pp.Suppress(pp.Keyword('position')) + pp.Suppress('(') + name + pp.Optional(grouped_by) + pp.Suppress(')') + comparison = ((column | ARITHMETIC) + comparator + (quoted | column | ARITHMETIC)).set_parse_action( + # pyrefly: ignore[implicit-any-lambda] + lambda t: UnresolvedComparisonNode(t[0], t[1], t[2]) ) - position_comparison = (position_call + comparator + position).set_parse_action(_position_comparison) - - comparison = (column + comparator + (number | quoted | column)).set_parse_action(_comparison) # pyrefly: ignore[implicit-any-lambda] existence = name.copy().set_parse_action(lambda t: UnresolvedNameNode(t[0])) - atom = ( - true_lit - | false_lit - | position_comparison - | comparison - | existence - | (pp.Suppress('(') + where_expr + pp.Suppress(')')) - ) + atom = true_lit | false_lit | comparison | existence | (pp.Suppress('(') + where_expr + pp.Suppress(')')) NOT = pp.CaselessKeyword('NOT').suppress() # pyrefly: ignore[implicit-any-lambda] @@ -199,6 +179,17 @@ def _named_rewrite(text: str, loc: int) -> str | None: ) +def _nested(node: Any) -> tuple[Any, ...]: + """What a where string nests through: a connective's operands, and the arithmetic on a comparison's sides.""" + if isinstance(node, UnresolvedComparisonNode): + return (node.left, node.right) + if isinstance(node, UnresolvedNameNode | ColumnNode | QuotedNode): + return () + if isinstance(node, AndNode | OrNode | NotNode | BooleanLiteralNode): + return where_children(node) + return children(node) + + @lru_cache(maxsize=4096) def parse_where(text: str) -> WhereNode | UnresolvedWhereNode: """Parse a where string into an AST, its leaves still unresolved. @@ -211,9 +202,10 @@ def parse_where(text: str) -> WhereNode | UnresolvedWhereNode: SchemaError: If *text* is not a where string of the language. A predictable mistake — ``&``/``|``/``~``/``!`` for a connective, a lone ``=`` — is named with its rewrite beside the grammar's own - complaint. + complaint. A side nesting past what an expression may is refused + as an expression is. """ return cast( 'WhereNode | UnresolvedWhereNode', - parse_text(_WHERE_GRAMMAR, text, 'where string', _named_rewrite, where_children, _DEEP_REWRITE), + parse_text(_WHERE_GRAMMAR, text, 'where string', _named_rewrite, _nested, _DEEP_REWRITE), ) diff --git a/src/math_spec/resolution.py b/src/math_spec/resolution.py index 20065626..56f782b3 100644 --- a/src/math_spec/resolution.py +++ b/src/math_spec/resolution.py @@ -45,9 +45,10 @@ with_children, ) from math_spec._where_parser import ( + ColumnNode, + QuotedNode, UnresolvedComparisonNode, UnresolvedNameNode, - UnresolvedPositionNode, UnresolvedWhereNode, parse_where, ) @@ -773,8 +774,6 @@ def where(self, node: WhereNode | UnresolvedWhereNode) -> WhereNode | Unresolved return node if isinstance(node, UnresolvedNameNode): return self._where_name(node) - if isinstance(node, UnresolvedPositionNode): - return self._position(node) if isinstance(node, UnresolvedComparisonNode): return self._comparison(node) if isinstance(node, NotNode): @@ -827,53 +826,104 @@ def _where_name(self, node: UnresolvedNameNode) -> WhereNode | UnresolvedWhereNo return VariableDefinedNode(node.name, ns.leaf_dims[node.name]) return node - def _position(self, node: UnresolvedPositionNode) -> DimensionPositionNode | UnresolvedPositionNode: + def _comparison(self, node: UnresolvedComparisonNode) -> WhereNode | UnresolvedWhereNode: + """``side side``, read for what each side is: a ``position()`` call, or a name against a literal or a column. + + The grammar admits any arithmetic on a side, and this is where the + language decides what it accepts there. + """ + if isinstance(node.left, FunctionCallNode) and node.left.name == 'position': + return self._position(node.left, node) + plain = self._plain(node) + return node if plain is None else self._plain_comparison(node, plain) + + def _plain(self, node: UnresolvedComparisonNode) -> _Plain | None: + """The comparison as ``name literal`` or ``name name``, or ``None`` with the refusal appended.""" + name, right = _side_name(node.left), node.right + value: float | str | None + quoted = isinstance(right, QuotedNode) + if isinstance(right, QuotedNode): + value = right.value + elif isinstance(right, ColumnNode): + value = right.shown + elif (literal := _literal(right)) is not None: + value = literal.value + else: + value = _side_name(right) + if name is None or value is None: + self.errors.append( + f'{self.context}: a where-comparison tests one name, relation column or position() against a ' + f'literal or a second column, and a side here is arithmetic, which is not in the language. ' + f'Precompute the test as a boolean parameter in data prep and test that.' + ) + return None + return _Plain(name, node.op, value, quoted) + + def _position( + self, call: FunctionCallNode, node: UnresolvedComparisonNode + ) -> DimensionPositionNode | UnresolvedComparisonNode: """``position(dim[, by=relation, within=columns]) i``: the name a dimension, ``by=`` a relation keyed over it.""" ns, context = self.ns, self.context - if node.dimension not in ns.dimensions: + shape = _position_shape(call) + if shape is None: + self.errors.append( + f'{context}: position() is written position([, by=, within=]), and this ' + f'call is not of that shape. It takes the dimension it counts along and nothing else beside by= and within=.' + ) + return node + dimension, by, into = shape + index = None if isinstance(node.right, ColumnNode | QuotedNode) else _literal(node.right) + if index is None or not index.value.is_integer(): + self.errors.append( + f'{context}: position({dimension}) is compared against an integer index, where 0 is first and a ' + f'negative number counts from the end. Write position({dimension}) {node.op} .' + ) + return node + position = int(index.value) + if dimension not in ns.dimensions: self.errors.append( f"{context}: position() counts along a dimension's coordinates, and " - f"'{node.dimension}' is {_declared_as(ns, node.dimension)}. " - f'{did_you_mean(node.dimension, ns.dimensions, label="Dimensions")}' + f"'{dimension}' is {_declared_as(ns, dimension)}. " + f'{did_you_mean(dimension, ns.dimensions, label="Dimensions")}' ) return node - if node.by is None: - return DimensionPositionNode(node.dimension, node.op, node.position) - if (problem := self._not_a_relation(node.by, 'position', 'by')) is not None: + if by is None: + return DimensionPositionNode(dimension, node.op, position) + if (problem := self._not_a_relation(by, 'position', 'by')) is not None: self.errors.append(problem) return node - call = f'position({node.dimension}, by={node.by})' - if node.into is None: + spelled = f'position({dimension}, by={by})' + if into is None: self.errors.append( - f'{context}: {call} leaves within= unsaid. {PARTITION_NAMES_ITS_GROUP} Write ' - f"position({node.dimension}, by={node.by}, within=) — the value columns of '{node.by}' " - f'are {list(ns.relations[node.by].values)}.' + f'{context}: {spelled} leaves within= unsaid. {PARTITION_NAMES_ITS_GROUP} Write ' + f"position({dimension}, by={by}, within=) — the value columns of '{by}' " + f'are {list(ns.relations[by].values)}.' ) return node - partition = self._partition(node.by, 'position', node.dimension, node.into) + partition = self._partition(by, 'position', dimension, into) if partition is None: return node - return DimensionPositionNode(node.dimension, node.op, node.position, partition) + return DimensionPositionNode(dimension, node.op, position, partition) - def _comparison(self, node: UnresolvedComparisonNode) -> WhereNode | UnresolvedWhereNode: + def _plain_comparison(self, node: UnresolvedComparisonNode, plain: _Plain) -> WhereNode | UnresolvedWhereNode: """``name literal``, or the one structural form ``relation relation``.""" ns, context = self.ns, self.context - value = node.value - left_name, _, left_column = node.name.partition('.') - if not node.quoted and isinstance(value, str): + value = plain.value + left_name, _, left_column = plain.name.partition('.') + if not plain.quoted and isinstance(value, str): right_name, _, right_column = value.partition('.') if (rhs_kind := ns.kind(right_name)) is not None: if rhs_kind == 'relation' and ns.kind(left_name) == 'relation': - left = self._relation_column(left_name, left_column or None, node.name, node.op) - right = self._relation_column(right_name, right_column or None, value, node.op) + left = self._relation_column(left_name, left_column or None, plain.name, plain.op) + right = self._relation_column(right_name, right_column or None, value, plain.op) if left is None or right is None: return node - if (refusal := _relation_pair_error(context, node, value, ns, left, right)) is not None: + if (refusal := _relation_pair_error(context, plain, value, ns, left, right)) is not None: self.errors.append(refusal) return node dims = tuple(ns.relations[left_name].dim(k) for k in ns.relations[left_name].key) - return RelationPairComparisonNode(left_name, left, right_name, right, node.op, dims) - self.errors.append(_declared_rhs_error(context, node, value, rhs_kind)) + return RelationPairComparisonNode(left_name, left, right_name, right, plain.op, dims) + self.errors.append(_declared_rhs_error(context, plain, value, rhs_kind)) return node kind = ns.kind(left_name) @@ -882,21 +932,21 @@ def _comparison(self, node: UnresolvedComparisonNode) -> WhereNode | UnresolvedW return node if left_column and kind != 'relation': self.errors.append( - f"{context}: '{node.name}' reads a column of '{left_name}', which is {_declared_as(ns, left_name)}. " + f"{context}: '{plain.name}' reads a column of '{left_name}', which is {_declared_as(ns, left_name)}. " f'Only a relation has columns.' ) return node column = None dtype: DeclaredDtype | None = None if kind == 'relation': - column = self._relation_column(left_name, left_column or None, node.name, node.op) + column = self._relation_column(left_name, left_column or None, plain.name, plain.op) if column is None: return node dtype = ns.dtypes[ns.relations[left_name].dim(column)] elif kind in ('parameter', 'dimension'): dtype = ns.dtypes[left_name] if dtype is not None: - typed = self._typed_literal(node, dtype) + typed = self._typed_literal(plain, dtype) if typed is None: return node value = typed @@ -904,13 +954,15 @@ def _comparison(self, node: UnresolvedComparisonNode) -> WhereNode | UnresolvedW match kind: case 'parameter': assert not isinstance(value, datetime.date) - return ParameterComparisonNode(left_name, node.op, value, ns.leaf_dims[left_name]) + return ParameterComparisonNode(left_name, plain.op, value, ns.leaf_dims[left_name]) case 'dimension': - return DimensionComparisonNode(left_name, node.op, value) + return DimensionComparisonNode(left_name, plain.op, value) case 'relation': assert column is not None shape = ns.relations[left_name] - return RelationComparisonNode(left_name, column, node.op, value, tuple(shape.dim(k) for k in shape.key)) + return RelationComparisonNode( + left_name, column, plain.op, value, tuple(shape.dim(k) for k in shape.key) + ) case 'variable': self.errors.append( f"{context}: where references variable '{left_name}'. A where " @@ -956,9 +1008,7 @@ def _relation_column(self, name: str, column: str | None, spelling: str, op: Pre return None return column - def _typed_literal( - self, node: UnresolvedComparisonNode, dtype: DeclaredDtype - ) -> float | str | datetime.date | None: + def _typed_literal(self, node: _Plain, dtype: DeclaredDtype) -> float | str | datetime.date | None: """The comparison's literal, checked against the declared dtype. Getting it wrong is silent: polars reads a datetime column against an @@ -1051,6 +1101,41 @@ def _without_sign(value: ArithmeticNode) -> ArithmeticNode: return value.operand if isinstance(value, UnaryOperatorNode) else value +class _Plain(NamedTuple): + """A where-comparison read as ``name literal`` or ``name name`` — the shape the dtype rules are written for. + + ``quoted`` says the right-hand side arrived in quotes, and so is a label + rather than a name to look up. + """ + + name: str + op: PredicateOperator + value: float | str + quoted: bool + + +def _side_name(side: ArithmeticNode | ColumnNode) -> str | None: + """The name a side of a where-comparison spells — bare or ``relation.column`` — or ``None`` where it is arithmetic.""" + if isinstance(side, NameNode): + return side.name + if isinstance(side, ColumnNode): + return side.shown + return None + + +def _position_shape(call: FunctionCallNode) -> tuple[str, str | None, tuple[str, ...] | None] | None: + """``(dim, by, within)`` off a ``position(...)`` call, or ``None`` where the call is not of that shape.""" + if len(call.args) != 1 or not isinstance(call.args[0], NameNode) or set(call.kwargs) - {'by', 'within'}: + return None + by, within = call.kwargs.get('by'), call.kwargs.get('within') + if by is not None and not isinstance(by, NameNode): + return None + if within is not None and not isinstance(within, NameNode | NameListNode): + return None + into = within.names if isinstance(within, NameListNode) else (within.name,) if within is not None else None + return call.args[0].name, by.name if by is not None else None, into + + def _literal(value: ArithmeticNode) -> NumberNode | None: """The number a literal names, its sign folded in — ``None`` where *value* is not one. @@ -1065,7 +1150,7 @@ def _literal(value: ArithmeticNode) -> NumberNode | None: return None -def _declared_rhs_error(context: str, node: UnresolvedComparisonNode, value: str, kind: str) -> str: +def _declared_rhs_error(context: str, node: _Plain, value: str, kind: str) -> str: """Why the right-hand side of a where-comparison may not name a declaration.""" comparison = f"'{node.name} {node.op} {value}'" if kind == 'parameter': @@ -1095,9 +1180,7 @@ def _declared_rhs_error(context: str, node: UnresolvedComparisonNode, value: str ) -def _relation_pair_error( - context: str, node: UnresolvedComparisonNode, other: str, ns: Namespace, left: str, right: str -) -> str | None: +def _relation_pair_error(context: str, node: _Plain, other: str, ns: Namespace, left: str, right: str) -> str | None: """Why two relation columns may not be compared, or ``None`` where they may. Both relations are read at their keys, so the keys must be over the same diff --git a/tests/test_parser.py b/tests/test_parser.py index e4cb1e77..96e1df03 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -37,9 +37,10 @@ parse_expression, ) from math_spec._where_parser import ( + ColumnNode, + QuotedNode, UnresolvedComparisonNode, UnresolvedNameNode, - UnresolvedPositionNode, parse_where, ) from math_spec.errors import SchemaError @@ -242,13 +243,14 @@ def test_a_list_the_grammar_cannot_read_is_refused_at_load(text): ) def test_scientific_notation_is_a_number(text, value): assert parse_expression(text) == NumberNode(value) - assert parse_where(f'p > {text}').value == value + assert parse_where(f'p > {text}').right == NumberNode(value), 'a where side is the expression grammar' @pytest.mark.parametrize('spelling', ['inf', '.inf']) def test_inf_is_a_literal(spelling): """Both spellings, since `bounds: {upper: .inf}` is how YAML writes it.""" assert parse_expression(f'p <= {spelling}').right == NumberNode(float('inf')) + assert parse_where(f'p < {spelling}').right == NumberNode(float('inf')), 'and a where reads it as one too' @pytest.mark.parametrize('name', ['inflow', 'influx', 'infeed', 'infrastructure', 'inf_max']) @@ -262,7 +264,7 @@ def test_a_name_may_begin_with_inf(name): [ pytest.param('True', BooleanLiteralNode, {'value': True}, id='a-literal'), pytest.param('p_max', UnresolvedNameNode, {'name': 'p_max'}, id='a-bare-name'), - pytest.param('p_max > 0', UnresolvedComparisonNode, {'op': '>', 'value': 0}, id='a-comparison'), + pytest.param('p_max > 0', UnresolvedComparisonNode, {'op': '>', 'right': NumberNode(0)}, id='a-comparison'), pytest.param('a AND b', AndNode, {}, id='and'), pytest.param('a OR b', OrNode, {}, id='or'), pytest.param('NOT a', NotNode, {}, id='not'), @@ -311,52 +313,89 @@ def test_conjuncts_does_not_split_or_or_not(text): @pytest.mark.parametrize( - ('text', 'value', 'quoted'), + ('text', 'right'), [ - ("g == 'wind'", 'wind', True), - ('g == "wind"', 'wind', True), - ("g == 'combined-cycle'", 'combined-cycle', True), - ("g == 'CCGT 400MW'", 'CCGT 400MW', True), - ("t > '2030-01-01'", '2030-01-01', True), - ("g == 'it\\'s'", "it's", True), - ('g == wind', 'wind', False), + ("g == 'wind'", QuotedNode('wind')), + ('g == "wind"', QuotedNode('wind')), + ("g == 'combined-cycle'", QuotedNode('combined-cycle')), + ("g == 'CCGT 400MW'", QuotedNode('CCGT 400MW')), + ("t > '2030-01-01'", QuotedNode('2030-01-01')), + ("g == 'it\\'s'", QuotedNode("it's")), + ('g == wind', NameNode('wind')), ], ids=['single', 'double', 'hyphen', 'space', 'date', 'escaped quote', 'bare'], ) -def test_a_quoted_right_hand_side_is_a_label(text, value, quoted): +def test_a_quoted_right_hand_side_is_a_label(text, right): """Quoting says "label, not name" (#460): unquoted, `combined-cycle` or `CCGT 400MW` was unsayable, and a bare word may name a declaration.""" node = parse_where(text) assert isinstance(node, UnresolvedComparisonNode) - assert node.value == value - assert node.quoted is quoted + assert node.right == right + + +def test_a_relation_column_is_named_with_a_dot(): + """`ends.bus0` is the one place the language names a column, and only a where side admits it.""" + assert parse_where('ends.bus0 != ends.bus1') == UnresolvedComparisonNode( + ColumnNode('ends', 'bus0'), '!=', ColumnNode('ends', 'bus1') + ) + with pytest.raises(SchemaError, match='Failed to parse where string'): + parse_where('ends.bus0 + 1 > 0') @pytest.mark.parametrize( - ('text', 'op', 'position', 'by'), + ('text', 'op', 'right', 'kwargs'), [ - ('position(snapshot) == 0', '==', 0, None), - ('position(snapshot) != 0', '!=', 0, None), - ('position(snapshot) > 0', '>', 0, None), - ('position(snapshot) <= -2', '<=', -2, None), - ('position(snapshot) == -1', '==', -1, None), - ('position(snapshot, by=period_of) == 0', '==', 0, 'period_of'), + ('position(snapshot) == 0', '==', NumberNode(0), {}), + ('position(snapshot) != 0', '!=', NumberNode(0), {}), + ('position(snapshot) > 0', '>', NumberNode(0), {}), + ('position(snapshot) <= -2', '<=', UnaryOperatorNode('-', NumberNode(2)), {}), + ('position(snapshot) == -1', '==', UnaryOperatorNode('-', NumberNode(1)), {}), + ('position(snapshot, by=period_of) == 0', '==', NumberNode(0), {'by': NameNode('period_of')}), + ( + 'position(snapshot, by=period_of, within=[a, b]) == 0', + '==', + NumberNode(0), + {'by': NameNode('period_of'), 'within': NameListNode(('a', 'b'))}, + ), ], - ids=['first', 'not first', 'after the first', 'band from the back', 'last', 'grouped'], + ids=['first', 'not first', 'after the first', 'band from the back', 'last', 'grouped', 'grouped within columns'], ) -def test_position_converts_a_dimension_to_where_a_row_sits(text, op, position, by): - """`position(dim)` is the left-hand side, so every comparator reads one way (#32).""" +def test_position_is_a_call_on_the_left_hand_side(text, op, right, kwargs): + """`position(dim)` is the left-hand side, so every comparator reads one way (#32), and it arrives as the + call the expression grammar builds; what its arguments may be is resolution's to say.""" node = parse_where(text) - assert isinstance(node, UnresolvedPositionNode) - assert node.dimension == 'snapshot' + assert isinstance(node, UnresolvedComparisonNode) + assert node.left == FunctionCallNode('position', (NameNode('snapshot'),), kwargs) assert node.op == op - assert node.position == position - assert node.by == by + assert node.right == right + + +@pytest.mark.parametrize( + 'text', + ['p > 0.5 * q', '(a + b) <= c', '-p < 1', 'sum(p, over=g) >= k', '2 < p', 'position(t) + 1 == 0'], + ids=[ + 'arithmetic-on-the-right', + 'a-bracketed-sum', + 'a-negated-name', + 'a-reduction', + 'a-literal-on-the-left', + 'position-inside-arithmetic', + ], +) +def test_a_side_is_any_arithmetic_to_the_grammar(text): + """The grammar hands both sides over bare; what the language admits on a side is decided in resolution, + where the schema is (`test_a_where_side_that_is_arithmetic_is_refused`).""" + assert isinstance(parse_where(text), UnresolvedComparisonNode) + + +def test_a_bracketed_predicate_is_still_a_predicate(): + """`(a > 0) AND b` groups a comparison; only `(a + b) <= c` brackets arithmetic.""" + assert isinstance(parse_where('(a > 0) AND b'), AndNode) -def test_a_position_is_not_confused_with_a_name(): - """`position` leads the alternation, so it is not read as a bare name.""" - assert isinstance(parse_where('position(t) == 0 AND p_max > 0'), AndNode) +def test_a_where_side_is_held_to_the_depth_an_expression_is(): + with pytest.raises(SchemaError, match='nests 121 deep'): + parse_where(' + '.join(['p'] * 120) + ' > 0') @pytest.mark.parametrize( @@ -379,8 +418,8 @@ def test_a_where_parse_failure_names_the_rewrite(text, rewrite): def test_a_legal_where_operator_is_never_diagnosed(): """`!=`, `<` and `>` are predicates here, unlike on the expression side, so no diagnosis may fire on them.""" - assert parse_where('status != 0') == UnresolvedComparisonNode('status', '!=', 0.0) - assert parse_where('p_max < 5') == UnresolvedComparisonNode('p_max', '<', 5.0) + assert parse_where('status != 0') == UnresolvedComparisonNode(NameNode('status'), '!=', NumberNode(0.0)) + assert parse_where('p_max < 5') == UnresolvedComparisonNode(NameNode('p_max'), '<', NumberNode(5.0)) def test_an_unrelated_parse_failure_says_nothing_about_positions(): diff --git a/tests/test_validation.py b/tests/test_validation.py index ee4ce219..4e8fefb3 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -553,6 +553,51 @@ def test_it_refuses(self, mask: str, fragments: list[str]): assert fragment in str(excinfo.value) +class TestAWhereSideIsReadInResolution: + """The grammar hands a comparison's sides over as arithmetic, and the language decides here what a side may be.""" + + @pytest.mark.parametrize( + ('where', 'fragments'), + [ + pytest.param( + 'c > 2 * k', ('a side here is arithmetic, which is not in the language',), id='arithmetic-on-a-side' + ), + pytest.param('2 < c', ('a side here is arithmetic',), id='a-literal-on-the-left'), + pytest.param('sum(c, over=g) >= k', ('a side here is arithmetic',), id='a-reduction-on-a-side'), + pytest.param( + 'position(g) == 1.5', + ('compared against an integer index', 'position(g) == '), + id='a-position-against-a-fraction', + ), + pytest.param('position(g) == c', ('compared against an integer index',), id='a-position-against-a-name'), + pytest.param( + 'position(g, h) == 0', + ('position() is written position([, by=, within=])',), + id='a-position-with-two-dimensions', + ), + pytest.param( + 'position(g, edge=1) == 0', + ('position() is written position([, by=, within=])',), + id='a-position-with-a-kwarg-it-lacks', + ), + pytest.param( + 'position(g, by=[lk, lk2]) == 0', + ('position() is written position([, by=, within=])',), + id='a-position-by-a-list', + ), + ], + ) + def test_a_side_the_language_does_not_admit_is_refused(self, where, fragments): + message = _refusal(**{'variables.p.where': where}) + for fragment in fragments: + assert fragment in message + + def test_a_signed_literal_and_inf_are_numbers_on_a_side(self): + """`-1` and `inf` are the expression grammar's literals, so a where reads them as it reads any number.""" + spec = _schema(**{'variables.p.where': 'c > -1 AND c < inf'}) + assert spec.variables['p'].where == 'c > -1 AND c < inf' + + class TestRulesDecidedWithoutData: """Every refusal the schema or the resolver makes with no data bound, one row each.""" diff --git a/tests/typesetting/test_golden.py b/tests/typesetting/test_golden.py index e93b0712..45d74650 100644 --- a/tests/typesetting/test_golden.py +++ b/tests/typesetting/test_golden.py @@ -132,14 +132,16 @@ def _rendered_trees() -> Iterator[object]: yield from resolved.expressions.values() -#: What resolution never hands the walk: the three nodes it types away, and the -#: three an expression only carries before names are resolved. The walk raises on -#: each rather than rendering it, so a fixture reaching one would be a bug in -#: resolution rather than a case worth committing output for. +#: What resolution never hands the walk: the four nodes a where carries before +#: its sides are read, and the three an expression only carries before names +#: are resolved. The walk raises on each rather than rendering it, so a fixture +#: reaching one would be a bug in resolution rather than a case worth +#: committing output for. UNRESOLVED = { 'UnresolvedNameNode', 'UnresolvedComparisonNode', - 'UnresolvedPositionNode', + 'ColumnNode', + 'QuotedNode', 'NameNode', 'NameListNode', 'KeywordNode', From 12154b964bc1153b5d6455134053f2ebe678c36c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 22:41:38 +0000 Subject: [PATCH 2/3] test: the parser test names the validation class that holds the refusals it points at Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EEeM2YoAk4Xr2uWB5qwMsH --- tests/test_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_parser.py b/tests/test_parser.py index 96e1df03..afce6c51 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -384,7 +384,7 @@ def test_position_is_a_call_on_the_left_hand_side(text, op, right, kwargs): ) def test_a_side_is_any_arithmetic_to_the_grammar(text): """The grammar hands both sides over bare; what the language admits on a side is decided in resolution, - where the schema is (`test_a_where_side_that_is_arithmetic_is_refused`).""" + where the schema is (`TestAWhereSideIsReadInResolution`).""" assert isinstance(parse_where(text), UnresolvedComparisonNode) From 18c3f69526f46e42ee3dc9597b72ba6b85e51215 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 18:06:31 +0000 Subject: [PATCH 3/3] chore(parser): the where depth measurement names the nodes it walks rather than Any `explicit-any` is an error on `src/math_spec` since #572, and that PR took only the fold's return type from #567, leaving `_nested` on this branch with a signature whose `Any` no longer even imports. `_ParsedWhere` is the union the measurement walks, which is #567's own answer for this file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V2SFmoxEF3SnaPKp7HbZTk --- src/math_spec/_where_parser.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/math_spec/_where_parser.py b/src/math_spec/_where_parser.py index 3f56aa65..ed9b0123 100644 --- a/src/math_spec/_where_parser.py +++ b/src/math_spec/_where_parser.py @@ -20,15 +20,21 @@ import pyparsing as pp -from math_spec._expression_parser import ARITHMETIC, NAME, children, parse_text -from math_spec.program import AndNode, BooleanLiteralNode, NotNode, OrNode, PredicateOperator, where_children +from math_spec._expression_parser import ARITHMETIC, NAME, ArithmeticNode, children, parse_text +from math_spec.program import ( + AndNode, + BooleanLiteralNode, + ConnectiveWhereNode, + NotNode, + OrNode, + PredicateOperator, + WhereNode, + where_children, +) if TYPE_CHECKING: from collections.abc import Callable - from math_spec._expression_parser import ArithmeticNode - from math_spec.program import WhereNode - # --------------------------------------------------------------------------- # AST nodes # --------------------------------------------------------------------------- @@ -84,6 +90,11 @@ class UnresolvedComparisonNode: #: leaves are still names the schema has not been asked about. UnresolvedWhereNode = UnresolvedNameNode | UnresolvedComparisonNode +#: Every node a parsed where string is built of: the connectives and literals, +#: the unresolved leaves, and the arithmetic and the two side nodes under a +#: comparison. What the depth measurement walks. +_ParsedWhere = WhereNode | UnresolvedWhereNode | ArithmeticNode | ColumnNode | QuotedNode + # --------------------------------------------------------------------------- # Grammar @@ -179,15 +190,15 @@ def _named_rewrite(text: str, loc: int) -> str | None: ) -def _nested(node: Any) -> tuple[Any, ...]: +def _nested(node: _ParsedWhere) -> tuple[_ParsedWhere, ...]: """What a where string nests through: a connective's operands, and the arithmetic on a comparison's sides.""" if isinstance(node, UnresolvedComparisonNode): return (node.left, node.right) - if isinstance(node, UnresolvedNameNode | ColumnNode | QuotedNode): - return () - if isinstance(node, AndNode | OrNode | NotNode | BooleanLiteralNode): + if isinstance(node, ArithmeticNode): + return children(node) + if isinstance(node, ConnectiveWhereNode): return where_children(node) - return children(node) + return () @lru_cache(maxsize=4096)