Raise DefinitionSyntaxError instead of AssertionError for an operator-only expression (#2124)#2322
Conversation
…-only expression
Parsing an expression that is just an operator, e.g. ureg('-') or ureg('*'),
left the eval-tree builder with no operand and tripped a bare `assert result is
not None`, surfacing an uninformative AssertionError to the caller.
Raise a DefinitionSyntaxError (as the surrounding malformed-token paths already
do) so callers get a proper pint error instead of an AssertionError. Valid
expressions and UndefinedUnitError for unknown unit names are unchanged.
Fixes hgrecco#2124
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merging this PR will degrade performance by 24.81%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_parse_expression[True-meter] |
59.6 µs | 84.6 µs | -29.53% |
| ❌ | WallTime | test_parse_expression[True-kilometer] |
61.6 µs | 86.4 µs | -28.74% |
| ❌ | WallTime | test_parse_expression[True-angstrom] |
61.7 µs | 86.6 µs | -28.72% |
| ❌ | WallTime | test_parse_expression[False-second] |
60.5 µs | 84.7 µs | -28.61% |
| ❌ | WallTime | test_parse_expression[False-minute] |
59.9 µs | 83.8 µs | -28.56% |
| ❌ | WallTime | test_parse_expression[True-minute] |
60.1 µs | 83.9 µs | -28.41% |
| ❌ | WallTime | test_parse_expression[False-millisecond] |
62.8 µs | 87.6 µs | -28.32% |
| ❌ | WallTime | test_parse_expression[False-meter] |
59.6 µs | 83.2 µs | -28.31% |
| ❌ | WallTime | test_parse_expression[False-angstrom] |
61.5 µs | 85.4 µs | -28.01% |
| ❌ | WallTime | test_parse_expression[True-millisecond] |
62.3 µs | 86.2 µs | -27.75% |
| ❌ | WallTime | test_parse_expression[True-second] |
60.1 µs | 83.1 µs | -27.73% |
| ❌ | WallTime | test_getitem[True-angstrom] |
36.8 µs | 50.5 µs | -27.18% |
| ❌ | WallTime | test_parse_expression[False-kilometer] |
62.3 µs | 85.4 µs | -27.02% |
| ❌ | WallTime | test_getitem[True-second] |
35.8 µs | 49 µs | -26.84% |
| ❌ | WallTime | test_getitem[False-angstrom] |
37.2 µs | 50.9 µs | -26.81% |
| ❌ | WallTime | test_op2[equal-keys5] |
42 µs | 57.3 µs | -26.78% |
| ❌ | WallTime | test_getitem[False-second] |
35.6 µs | 48.5 µs | -26.59% |
| ❌ | WallTime | test_finding_meter_getitem |
34.9 µs | 47.5 µs | -26.5% |
| ❌ | WallTime | test_getitem[True-meter] |
34.7 µs | 47.2 µs | -26.4% |
| ❌ | WallTime | test_getitem[True-minute] |
35.6 µs | 48.1 µs | -25.96% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing vineethsaivs:fix/eval-tree-missing-operand (62fdd54) with master (7a927b4)
Footnotes
-
448 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Fixes #2124.
Parsing an expression that is only an operator raised a bare
AssertionErrorrather than a pint error:In
pint_eval._build_eval_tree, an operator with no operand leavesresult is None, which trippedassert result is not None. The surrounding malformed-token paths already raiseDefinitionSyntaxError, so this does the same:Valid expressions are unchanged (
ureg('3 m / s'),ureg('m**2'), ...), and an unknown unit name still raisesUndefinedUnitError(ureg('foobar')).Added
test_build_eval_tree_missing_operand_raises_definition_syntax_error(parametrized over both tokenizers and several operators); it fails onmaster(AssertionError) and passes with this change. All 145 tests intest_pint_eval.pypass.Disclosure: prepared with AI assistance; reviewed by me and I can explain every line.