asteval errors are not flushed after raising

[schachmett]

Description

With parameter.set(expr="foo"), if the expression raises an exception in asteval, this error is stored in parameter._expr_eval.errors. Now, the function https://github.com/lmfit/lmfit-py/blob/6c87262fcfd3c361b197c6769852f76366113246/lmfit/parameter.py#L18-L21 gets called from parameter.__set_expression(val) and checks if the length of this errors variable is longer than 0 and if it is, the exception is raised.

However, if you try to set the expression for this parameter again (to a VALID expression), the Parameter._expr_eval.errors variable will still contain the first exception and so, check_ast_errors() will always raise an Exception. This should not be the expected behaviour, for example if lmfit is used from a GUI and the user tries to enter a valid expression after first entering an invalid one (where the exception was caught).

I will submit a PR where check_ast_errors() flushes the parameter._expr_eval.errors before trying to set a new expression.

Version information

Python: 3.6.5 (default, Apr 1 2018, 05:46:30) [GCC 7.3.0] lmfit: 0.9.11, scipy: 1.1.0, numpy: 1.15.0, asteval: 0.9.12, uncertainties: 3.0.2, six: 1.11.0

Minimal example

import lmfit

def set_constraints(paramname, expr):      
#        params[paramname]._expr_eval.error.clear() # this would be the dirty fix
        try:
            params[paramname].set(expr=expr)
            print("expr was valid")
        except SyntaxError:
            params[paramname].set(expr="")
            print("expr was not valid")
model = lmfit.models.PseudoVoigtModel()
params = model.make_params()
set_constraints("amplitude", expr="sigma * 2")
set_constraints("amplitude", expr="fail *")
set_constraints("amplitude", expr="sigma * 2")

produces:

expr was valid
expr was not valid
expr was not valid

If you uncomment the "dirty fix", it produces

expr was valid
expr was not valid
expr was valid

[schachmett]

Seeing that my PR failed travis-ci and I am not very experienced with your project, another suggestion would be to provide a public method that checks if an expression is valid without already setting it by trying to parse it. This way, also NameErrors (if parameter names dont exist) are caught.

[newville]

@schachmett sorry for the delay: I was traveling. I agree that this looks like a real problem needing a solution and that #487 might be a good start of that solution. I'll try to look into this week too.

[schachmett]

@newville no problem and thank you. At the moment, I am using something like

def set_expr(param, expr):
    evaluator = param._expr_eval
    evaluator.error.clear()
    isvalid = True
    try:
        ast_expr = evaluator.parse(expr)
        evaluator(ast_expr, show_errors=False)
    except (SyntaxError, NameError):    # Possibly catch more errors that evaluator might throw
        expr = ""
        isvalid = False
    param.set(expr=expr)
    return isvalid

where expr is the algebraic expression I wanto to test for Parameter param. isvalid is returned to see if the operation was successful.

I think a good behavior would be if Parameter.set() would raise the exception inside evaluator.error after clearing it.