Downstream consumers of the submodule's main (relevant) utility function can't just cleanly import the error type. They have to access it either as an attribute of one of the classes (ExpressionEvaluator or its child EquationEvaluator), or as an attribute of the "function" eval_equation() (which is actually an instance of EquationEvaluator, but 🤫).
This is more than a little weird as Python exception types go. Plus, if someone just lets Sopel's built-in error handler catch the exception, you get "Unexpected Error" in the output, because the class name is just "Error". Not very descriptive, that.
Proposal
Let's move the ExpressionEvaluator.Error type out into its own standalone class. In fact, it should probably become a full error hierarchy to cover the range of different error cases available. (WIP tests for the tools.calculation module currently have to substring-match the exception message to make sure the "right" Error is being raised.)
I've toyed with ideas for keeping the ExpressionEvaluator.Error name around for a while in case anyone else is using it (like I've just proposed doing in #2507), but am not dead set on doing so. The new standalone error type (hierarchy root, tentatively CalculationError) could start its life as a subclass of the old one, and then switch to subclassing Exception directly in the next major version. There won't be an effective hook point to show any deprecation warnings, though, so anyone using code that relies on the old (current) name will have to read release notes. 🤷♂️
Because the
Errorraised byExpressionEvaluatoris defined here, inside theExpressionEvaluatorclass itself:https://github.com/sopel-irc/sopel/blob/274470ec29429a26bd0faed12984ce6ac09cf949/sopel/tools/calculation.py#L22-L24
Downstream consumers of the submodule's main (relevant) utility function can't just cleanly import the error type. They have to access it either as an attribute of one of the classes (
ExpressionEvaluatoror its childEquationEvaluator), or as an attribute of the "function"eval_equation()(which is actually an instance ofEquationEvaluator, but 🤫).This is more than a little weird as Python exception types go. Plus, if someone just lets Sopel's built-in error handler catch the exception, you get "Unexpected Error" in the output, because the class name is just "Error". Not very descriptive, that.
Proposal
Let's move the
ExpressionEvaluator.Errortype out into its own standalone class. In fact, it should probably become a full error hierarchy to cover the range of different error cases available. (WIP tests for thetools.calculationmodule currently have to substring-match the exception message to make sure the "right"Erroris being raised.)I've toyed with ideas for keeping the
ExpressionEvaluator.Errorname around for a while in case anyone else is using it (like I've just proposed doing in #2507), but am not dead set on doing so. The new standalone error type (hierarchy root, tentativelyCalculationError) could start its life as a subclass of the old one, and then switch to subclassingExceptiondirectly in the next major version. There won't be an effective hook point to show any deprecation warnings, though, so anyone using code that relies on the old (current) name will have to read release notes. 🤷♂️