Challenging to diagnose bad expressions when using numerical comparison operators.

[klarose]

I have an expression where one side of a comparison operator fails (e.g. undeclared reference). The result is a "no matching overload" error, which isn't very helpful. I expect to see the original failure bubble up to the top of the evaluation.

E.g.

python3 -m celpy -n 'bad < 113.0'
ERROR: <input>:1:1 found no matching overload for 'relation_lt' applied to '(<class 'celpy.evaluation.CELEvalError'>, <class 'celpy.celtypes.DoubleType'>)'
    | bad < 113.0
    | ^

While the offending part of the expression is still in the output, it's still fairly misleading, and could cause code trying to customize error handling to do the wrong thing.

I took a quick gander where the failure occurs, and it seems to be that the builtin comparison operators don't have an overload for the CELEvalError class. Many of the others do. I'm not sure if this was a miss or on purpose.

Either way, hacking this in seemed to fix the problem:

diff --git a/src/celpy/evaluation.py b/src/celpy/evaluation.py
index 799b05c..8545a02 100644
--- a/src/celpy/evaluation.py
+++ b/src/celpy/evaluation.py
@@ -228,10 +228,22 @@ class CELEvalError(Exception):
     def __rpow__(self, other: Any) -> "CELEvalError":
         return self
 
+    def __lt__(self, other: Any) -> "CELEvalError":
+        raise self
+
+    def __le__(self, other: Any) -> "CELEvalError":
+        raise self
+
+    def __gt__(self, other: Any) -> "CELEvalError":
+        raise self
+
+    def __ge__(self, other: Any) -> "CELEvalError":
+        raise self
+
     def __eq__(self, other: Any) -> bool:
         if isinstance(other, CELEvalError):
             return self.args == other.args
-        return NotImplemented
+        raise self
 
     def __call__(self, *args: Any) -> "CELEvalError":
         return self

I can submit a PR if there's interest.

[slott56]

It's important to benchmark against the Go version of CEL to be sure what it does with these kinds of things.

A PR is welcome.

[klarose]

The go playground shows:

failed to compile the CEL expression: ERROR: <input>:8:2: undeclared reference to 'bad' (in container '')
 | 
 | ^

playground link

[klarose]

I'll try to put up a PR in the next few days. I'm a bit bogged down now. Feel free to poke me if it looks like I forgot. :)

[Quantum0uasar]

Checking in on this — @klarose did you get a chance to open the PR? The issue is still open and it would be useful to have better error propagation here. If not, I'd be happy to take a look.

The core of the fix would be catching the upstream CELEvalError (from the undeclared reference resolution) in the comparison operator handler and re-raising it with context, rather than masking it with a generic "no matching overload" message. Should be a relatively contained change in the evaluation layer.

[klarose]

Checking in on this — @klarose did you get a chance to open the PR? The issue is still open and it would be useful to have better error propagation here. If not, I'd be happy to take a look.

The core of the fix would be catching the upstream CELEvalError (from the undeclared reference resolution) in the comparison operator handler and re-raising it with context, rather than masking it with a generic "no matching overload" message. Should be a relatively contained change in the evaluation layer.

Sorry, I haven't really had much of a chance. If you're in a pinch I can try to fit it in some time this week.