From 65fde09dc02b0f793af4045bdf810756792ae1f3 Mon Sep 17 00:00:00 2001 From: Hasnain Habib Sayed Date: Sat, 7 Mar 2026 22:53:11 +0600 Subject: [PATCH 1/3] Improve parser error message for missing units in equations A more explicit error message when a user forgets to specify the units in an equation. It intercepts the generic `Expected end of text` error from pyparsing and now suggests checking for an omitted unit or missing colon (:), improving the debugging experience for syntax errors. --- brian2/equations/equations.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/brian2/equations/equations.py b/brian2/equations/equations.py index 9b987e587..5f518b4fc 100644 --- a/brian2/equations/equations.py +++ b/brian2/equations/equations.py @@ -386,14 +386,17 @@ def parse_string_equations(eqns): try: parsed = EQUATIONS.parse_string(eqns, parse_all=True) except ParseException as p_exc: - raise EquationError( - "Parsing failed: \n" - + str(p_exc.line) - + "\n" - + " " * (p_exc.column - 1) - + "^\n" - + str(p_exc) - ) from p_exc + error_msg = ("Parsing failed: \n" + + str(p_exc.line) + "\n" + + " " * (p_exc.column - 1) + "^\n" + + str(p_exc)) + if p_exc.msg == "Expected end of text": + error_msg += "\n(Check if you omitted the unit for a parameter or equation, or if there is a syntax error. " + if ':' not in p_exc.line: + error_msg += "Possibly missing a colon (':') to specify the unit.)" + else: + error_msg += ")" + raise EquationError(error_msg) from p_exc for eq in parsed: eq_type = eq.getName() eq_content = dict(eq.items()) From dda4792d73e4eea27435e56f771886af892c81ef Mon Sep 17 00:00:00 2001 From: Hasnain Habib Sayed Date: Wed, 11 Mar 2026 12:39:03 +0000 Subject: [PATCH 2/3] Enhance error diagnostics for equation parsing and add comment annotations in tests --- brian2/equations/equations.py | 295 ++++++++++++++++++++++++++++----- brian2/tests/test_equations.py | 53 ++++++ 2 files changed, 311 insertions(+), 37 deletions(-) diff --git a/brian2/equations/equations.py b/brian2/equations/equations.py index 5f518b4fc..8e435a57c 100644 --- a/brian2/equations/equations.py +++ b/brian2/equations/equations.py @@ -134,6 +134,216 @@ class EquationError(Exception): pass +def _count_colons_outside_parens(s): + """Count ':' characters not nested inside parentheses.""" + depth = count = 0 + for ch in s: + if ch == "(": + depth += 1 + elif ch == ")": + depth = max(depth - 1, 0) + elif ch == ":" and depth == 0: + count += 1 + return count + + +def _paren_mismatch_msg(expr): + """Return a parenthesis-mismatch diagnostic, or ``None`` if balanced. + + Uses depth-tracking so that ``)(`` (equal counts but wrong order) is + correctly flagged rather than silently treated as balanced. + """ + depth = extra_close = 0 + for ch in expr: + if ch == "(": + depth += 1 + elif ch == ")": + if depth: + depth -= 1 + else: + extra_close += 1 + unclosed = depth + if not unclosed and not extra_close: + return None + + n_open, n_close = expr.count("("), expr.count(")") + if not extra_close: + noun = "paren" if unclosed == 1 else "parens" + return ( + f"Unmatched parenthesis — {n_open} opening '(' but only " + f"{n_close} closing ')'. Add {unclosed} closing {noun} to balance." + ) + if not unclosed: + noun = "paren" if extra_close == 1 else "parens" + return ( + f"Unmatched parenthesis — {n_close} closing ')' but only " + f"{n_open} opening '('. Remove {extra_close} extra {noun} or add matching '('." + ) + # Mixed: stray ')' before its '(' and unclosed '(' at the end (e.g. ')(') + return ( + f"Mismatched parentheses — {unclosed} unclosed '(' and " + f"{extra_close} unmatched ')'. Check parentheses order and count." + ) + + +def _multiword_unit_hint(unit_str): + """Return a hint if *unit_str* has bare multiple words, else ``''``.""" + words = unit_str.strip().split() + if len(words) > 1 and not any(op in unit_str for op in ("/", "*")): + return ( + f"Unit '{unit_str}' looks invalid — multiple words without " + f"operators. Did you mean '{words[0]}' or '{' * '.join(words)}'?" + ) + return "" + + +_EQUATION_FORMS = ( + " d/dt = : (differential equation)\n" + " = : (subexpression)\n" + " : (parameter)" +) + + +def _diagnose_single_line(line, line_num): + """ + Targeted diagnostic for a single equation line that failed to parse. + Returns ``None`` if the line parses successfully. + """ + try: + EQUATION.parse_string(line, parse_all=True) + return None + except ParseException: + pass + + code = line.split("#")[0].strip() + if not code: + return None + + def _fmt(title, suggestion): + return f" Line {line_num}: {title}\n {line}\n {suggestion}" + + # Only check parens in the expression part (before ':') so that + # flag parens such as '(const)' after the unit don't cause false + # positives. + expr_part = code.split(":")[0] if ":" in code else code + paren_msg = _paren_mismatch_msg(expr_part) + if paren_msg: + return _fmt(paren_msg, "Fix the parentheses and retry.") + + diff_m = re.match(r"d(\w+)/d(\w*)", code) + if diff_m and diff_m.group(2) != "t": + var, bad = diff_m.group(1), diff_m.group(2) + label = "Incomplete" if bad == "" else "Invalid" + return _fmt( + f"{label} differential operator 'd{var}/d{bad}'.", + f"The time derivative must be 'd{var}/dt'.", + ) + + colons = _count_colons_outside_parens(code) + if colons > 1: + return _fmt( + f"Found {colons} ':' separators, expected exactly 1.", + "Each equation must have the form: : ", + ) + if colons == 0: + example = ( + ( + "Every equation needs a unit after ':', e.g.:\n" + " dv/dt = -v / tau : volt\n" + " x = 2 * y : 1 (dimensionless)" + ) + if "=" in code + else ("Parameters: : Equations: = : ") + ) + return _fmt("Missing ':' unit separator.", example) + + lhs, _, rhs = code.partition(":") + rhs = rhs.strip() + if not rhs: + return _fmt( + "Missing unit after ':'.", + "Specify a unit, e.g. 'volt', 'second', or '1' for dimensionless.", + ) + if "=" in lhs and not lhs.split("=", 1)[1].strip(): + return _fmt( + "Empty expression after '='.", + "Provide a right-hand side expression, e.g.: x = 2 * y : volt", + ) + + unit_for_check = re.sub(r"\([^)]*\)\s*$", "", rhs).strip() + unit_hint = _multiword_unit_hint(unit_for_check) + if unit_hint: + return _fmt(unit_hint, "") + + return _fmt("Cannot parse this equation.", f"Expected one of:\n{_EQUATION_FORMS}") + + +_VAR_START_RE = re.compile(r"d(\w+)\s*/\s*dt\b|(\w+)\s*[=:]") +_MULTI_WHITESPACE_RE = re.compile(r"\s{2,}") + + +def _extract_comments(eqns_str): + """ + Map inline ``#``-comments to the variable defined on the same logical + line. Returns ``{varname: comment_text}``. + """ + result = {} + current_var = None + pending = [] + + for raw in eqns_str.split("\n"): + stripped = raw.strip() + if not stripped: + if current_var and pending: + result[current_var] = " ".join(pending) + current_var, pending = None, [] + continue + code, _, comment = stripped.partition("#") + code = code.strip() + if code and (m := _VAR_START_RE.match(code)): + if current_var and pending: + result[current_var] = " ".join(pending) + current_var, pending = m.group(1) or m.group(2), [] + if comment: + pending.append(comment.strip()) + + if current_var and pending: + result[current_var] = " ".join(pending) + return result + + +def _diagnose_parse_error(eqns_str, parse_exception): + """ + Try each non-empty line individually and collect targeted diagnostics. + Falls back to the raw ``ParseException`` when nothing specific is found. + """ + diagnostics = [] + for line_num, raw in enumerate(eqns_str.split("\n"), start=1): + s = raw.strip() + if not s or s.startswith("#"): + continue + diag = _diagnose_single_line(s, line_num) + if diag is not None: + diagnostics.append(diag) + + if not diagnostics: + # pyparsing columns are 1-based; clamp to 0 to avoid negative repeats. + col = max(0, parse_exception.column - 1) + return ( + f"Parsing failed:\n" + f" {parse_exception.line}\n" + f" {' ' * col}^\n" + f" {parse_exception}" + ) + return "Equation parsing failed.\n" + "\n".join(diagnostics) + + +def _diagnose_expression_error(varname, expr_str, syntax_error): + """Helpful message when an expression is syntactically invalid Python.""" + prefix = f"Syntax error in the expression for variable '{varname}':\n {expr_str}\n" + return prefix + (_paren_mismatch_msg(expr_str) or str(syntax_error)) + + def check_identifier_basic(identifier): """ Check an identifier (usually resulting from an equation string provided by @@ -232,7 +442,7 @@ def check_identifier_functions(identifier): def check_identifier_constants(identifier): """ - Make sure that identifier names do not clash with function names. + Make sure that identifier names do not clash with constant names. """ if identifier in DEFAULT_CONSTANTS: raise SyntaxError( @@ -241,8 +451,7 @@ def check_identifier_constants(identifier): ) -_base_units_with_alternatives = None -_base_units = None +_base_units_with_alternatives = _base_units = None def dimensions_and_type_from_string(unit_string): @@ -270,8 +479,7 @@ def dimensions_and_type_from_string(unit_string): # Lazy import to avoid circular dependency from brian2.core.namespace import DEFAULT_UNITS - global _base_units_with_alternatives - global _base_units + global _base_units_with_alternatives, _base_units if _base_units_with_alternatives is None: base_units_for_dims = {} for unit_name, unit in reversed(DEFAULT_UNITS.items()): @@ -358,7 +566,6 @@ def dimensions_and_type_from_string(unit_string): f"has type {type(evaluated_unit)} instead." ) - # No error has been raised, all good return evaluated_unit.dim, FLOAT @@ -386,41 +593,44 @@ def parse_string_equations(eqns): try: parsed = EQUATIONS.parse_string(eqns, parse_all=True) except ParseException as p_exc: - error_msg = ("Parsing failed: \n" + - str(p_exc.line) + "\n" + - " " * (p_exc.column - 1) + "^\n" + - str(p_exc)) - if p_exc.msg == "Expected end of text": - error_msg += "\n(Check if you omitted the unit for a parameter or equation, or if there is a syntax error. " - if ':' not in p_exc.line: - error_msg += "Possibly missing a colon (':') to specify the unit.)" - else: - error_msg += ")" - raise EquationError(error_msg) from p_exc + raise EquationError(_diagnose_parse_error(eqns, p_exc)) from p_exc + + comments = _extract_comments(eqns) + for eq in parsed: eq_type = eq.getName() eq_content = dict(eq.items()) - # Check for reserved keywords identifier = eq_content["identifier"] - # Convert unit string to Unit object try: dims, var_type = dimensions_and_type_from_string(eq_content["unit"]) except ValueError as ex: - raise EquationError( - f"Error parsing the unit specification for variable '{identifier}'." - ) from ex + hint = _multiword_unit_hint(eq_content["unit"]) + msg = f"Error parsing the unit specification for variable '{identifier}': {ex}" + if hint: + msg += f"\n {hint}" + raise EquationError(msg) from ex expression = eq_content.get("expression") if expression is not None: - # Replace multiple whitespaces (arising from joining multiline - # strings) with single space - p = re.compile(r"\s{2,}") - expression = Expression(p.sub(" ", expression)) + expr_str = _MULTI_WHITESPACE_RE.sub(" ", expression) + try: + expression = Expression(expr_str) + except SyntaxError as syn_exc: + raise EquationError( + _diagnose_expression_error(identifier, expr_str, syn_exc) + ) from syn_exc flags = list(eq_content.get("flags", [])) + comment = comments.get(identifier) equation = SingleEquation( - eq_type, identifier, dims, var_type=var_type, expr=expression, flags=flags + eq_type, + identifier, + dims, + var_type=var_type, + expr=expression, + flags=flags, + comment=comment, ) if identifier in equations: @@ -457,10 +667,17 @@ class SingleEquation(Hashable, CacheKey): context. """ - _cache_irrelevant_attributes = {"update_order"} + _cache_irrelevant_attributes = {"update_order", "comment"} def __init__( - self, type, varname, dimensions, var_type=FLOAT, expr=None, flags=None + self, + type, + varname, + dimensions, + var_type=FLOAT, + expr=None, + flags=None, + comment=None, ): self.type = type self.varname = varname @@ -478,10 +695,9 @@ def __init__( "Differential equations can only define floating point variables" ) self.expr = expr - if flags is None: - self.flags = [] - else: - self.flags = list(flags) + self.flags = list(flags) if flags is not None else [] + + self.comment = comment # will be set later in the sort_subexpressions method of Equations self.update_order = -1 @@ -725,6 +941,7 @@ def _substitute(self, replacements): var_type=eq.var_type, expr=Expression(new_code), flags=eq.flags, + comment=eq.comment, ) else: new_equations[new_varname] = SingleEquation( @@ -733,6 +950,7 @@ def _substitute(self, replacements): dimensions=eq.dim, var_type=eq.var_type, flags=eq.flags, + comment=eq.comment, ) return new_equations @@ -1189,9 +1407,7 @@ def check_units(self, group, run_namespace): check_dimensions(str(eq.expr), self.dimensions[var], all_variables) except DimensionMismatchError as ex: raise DimensionMismatchError( - "Inconsistent units in " - f"subexpression {eq.varname}:" - f"\n%{ex.desc}", + f"Inconsistent units in subexpression {eq.varname}:\n{ex.desc}", *ex.dims, ) from ex else: @@ -1383,7 +1599,12 @@ def extract_constant_subexpressions(eqs): flags = set(eq.flags) - {"constant over dt"} without_const_subexpressions.append( SingleEquation( - PARAMETER, eq.varname, eq.dim, var_type=eq.var_type, flags=flags + PARAMETER, + eq.varname, + eq.dim, + var_type=eq.var_type, + flags=flags, + comment=eq.comment, ) ) const_subexpressions.append(eq) diff --git a/brian2/tests/test_equations.py b/brian2/tests/test_equations.py index ed985c465..a6ffdc9b9 100644 --- a/brian2/tests/test_equations.py +++ b/brian2/tests/test_equations.py @@ -662,6 +662,57 @@ def test_ipython_pprint(): sys.stdout = old_stdout +@pytest.mark.codegen_independent +def test_improved_error_messages(): + """Test that malformed equations produce clear, targeted diagnostics.""" + + # Missing colon (unit separator) + with pytest.raises(EquationError, match=r"Missing ':' unit separator"): + parse_string_equations("dv/dt = -v / tau volt") + + # Malformed differential operator + with pytest.raises(EquationError, match=r"Incomplete differential operator"): + parse_string_equations("dv/d = -v / tau : 1") + + with pytest.raises(EquationError, match=r"Invalid differential operator"): + parse_string_equations("dv/dx = -v / tau : 1") + + # Unmatched parenthesis (open) + with pytest.raises(EquationError, match=r"Unmatched parenthesis"): + parse_string_equations("dv/dt = -v / (tau : volt") + + # Double colon + with pytest.raises(EquationError, match=r"Found 2 ':' separators"): + parse_string_equations("dv/dt = -v / tau : 1 : volt") + + # Two-word unit without operator + with pytest.raises(EquationError, match=r"looks invalid"): + parse_string_equations("dv/dt = -v / tau : volt second") + + # Missing unit after colon + with pytest.raises(EquationError, match=r"Missing unit after ':'"): + parse_string_equations("dv/dt = -v / tau :") + + +@pytest.mark.codegen_independent +def test_comment_annotations(): + """Test that inline comments are captured as annotations on equations.""" + eqs = parse_string_equations( + """ + dv/dt = -(v + ge) / tau : volt # membrane potential + dge/dt = -ge / tau_ge : volt # excitatory conductance + f : Hz # driving frequency + """ + ) + assert eqs["v"].comment == "membrane potential" + assert eqs["ge"].comment == "excitatory conductance" + assert eqs["f"].comment == "driving frequency" + + # Equations without comments should have None + eqs2 = parse_string_equations("dv/dt = -v / tau : volt") + assert eqs2["v"].comment is None + + if __name__ == "__main__": test_utility_functions() test_identifier_checks() @@ -676,3 +727,5 @@ def test_ipython_pprint(): test_extract_subexpressions() test_repeated_construction() test_str_repr() + test_improved_error_messages() + test_comment_annotations() From 45b6f0d7615090afb762e80ab2410ba3d016d0d6 Mon Sep 17 00:00:00 2001 From: Hasnain Habib Sayed Date: Thu, 26 Mar 2026 13:19:02 +0000 Subject: [PATCH 3/3] Improve error diagnostics for missing unit separators in equations --- brian2/equations/equations.py | 28 ++++++++++++++++------ brian2/tests/test_equations.py | 43 +++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/brian2/equations/equations.py b/brian2/equations/equations.py index 8e435a57c..3dbafc700 100644 --- a/brian2/equations/equations.py +++ b/brian2/equations/equations.py @@ -329,17 +329,31 @@ def _diagnose_parse_error(eqns_str, parse_exception): if not diagnostics: # pyparsing columns are 1-based; clamp to 0 to avoid negative repeats. col = max(0, parse_exception.column - 1) - return ( - f"Parsing failed:\n" - f" {parse_exception.line}\n" - f" {' ' * col}^\n" - f" {parse_exception}" - ) + # parse_exception.line can be empty/None when ZeroOrMore is involved. + raw_line = parse_exception.line or eqns_str.strip().split("\n")[0] + return f"Parsing failed:\n {raw_line}\n {' ' * col}^\n {parse_exception}" return "Equation parsing failed.\n" + "\n".join(diagnostics) def _diagnose_expression_error(varname, expr_str, syntax_error): - """Helpful message when an expression is syntactically invalid Python.""" + """Helpful message when an expression is syntactically invalid Python. + + When a line is missing its ':' unit separator, ``EXPRESSION`` can greedily + swallow the next equation line (e.g. ``dge/dt = ...``), producing a + cryptic SyntaxError. Detect this and emit a targeted missing-colon hint + instead. + """ + # Heuristic: if the expression contains '/dt' or looks like it ate a + # following equation line ('d/dt' or ' ='), the real problem is + # a missing ':' on the current line. + if re.search(r"/dt\b", expr_str) or re.search(r"\b\w+\s*=", expr_str): + return ( + f"Missing ':' unit separator in the equation for variable '{varname}'.\n" + f" The expression looks like it accidentally absorbed the next line:\n" + f" {expr_str.strip()!r}\n" + f" Add ':' before the unit, e.g.:\n" + f" d{varname}/dt = : " + ) prefix = f"Syntax error in the expression for variable '{varname}':\n {expr_str}\n" return prefix + (_paren_mismatch_msg(expr_str) or str(syntax_error)) diff --git a/brian2/tests/test_equations.py b/brian2/tests/test_equations.py index a6ffdc9b9..01fa870f9 100644 --- a/brian2/tests/test_equations.py +++ b/brian2/tests/test_equations.py @@ -666,10 +666,26 @@ def test_ipython_pprint(): def test_improved_error_messages(): """Test that malformed equations produce clear, targeted diagnostics.""" - # Missing colon (unit separator) + # Missing colon (unit separator) — single line with pytest.raises(EquationError, match=r"Missing ':' unit separator"): parse_string_equations("dv/dt = -v / tau volt") + # Missing colon in a multiline block — EXPRESSION greedily absorbs the next + # line, which is detected in _diagnose_expression_error. + with pytest.raises( + EquationError, match=r"Missing ':' unit separator in the equation" + ): + parse_string_equations( + """ + dv/dt = -v / tau volt + dge/dt = -ge / tau_ge : volt + """ + ) + + # Missing colon on a parameter line (no '=' involved) + with pytest.raises(EquationError, match=r"Missing ':' unit separator"): + parse_string_equations("f Hz") + # Malformed differential operator with pytest.raises(EquationError, match=r"Incomplete differential operator"): parse_string_equations("dv/d = -v / tau : 1") @@ -677,6 +693,10 @@ def test_improved_error_messages(): with pytest.raises(EquationError, match=r"Invalid differential operator"): parse_string_equations("dv/dx = -v / tau : 1") + # 'dt' typo (dv/d) must NOT be masked as a missing-unit hint + with pytest.raises(EquationError, match=r"Incomplete differential operator"): + parse_string_equations("dv/d = -v / tau : 1") + # Unmatched parenthesis (open) with pytest.raises(EquationError, match=r"Unmatched parenthesis"): parse_string_equations("dv/dt = -v / (tau : volt") @@ -694,6 +714,26 @@ def test_improved_error_messages(): parse_string_equations("dv/dt = -v / tau :") +@pytest.mark.codegen_independent +def test_parse_error_false_positives(): + """Valid equations must never trigger error-message diagnostics.""" + valid = [ + "dv/dt = -v / tau : volt", + "x : volt", + "x = 2*v : volt", + "dv/dt = -v / tau : 1", + "dv/dt = -v / tau : volt # with a comment", + """ + dv/dt = -(v + ge) / tau : volt + dge/dt = -ge / tau_ge : volt + f : Hz + """, + ] + for eq_str in valid: + # Should not raise any exception + parse_string_equations(eq_str) + + @pytest.mark.codegen_independent def test_comment_annotations(): """Test that inline comments are captured as annotations on equations.""" @@ -728,4 +768,5 @@ def test_comment_annotations(): test_repeated_construction() test_str_repr() test_improved_error_messages() + test_parse_error_false_positives() test_comment_annotations()