No error raised when setting an incorrect expression to a parameter

[Marc Lenertz]

Hello

I’m currently developing a graphical interface for changing my fit parameters and found some problems with the parameters’ expressions.

When I set an incorrect expression to a parameter only syntax errors will raise an exception. Name errors will only occur once I access the parameter’s value. Is this a known behavior or a bug? The only reason I see to keep this behavior is because you can add parameters that are linked together in the order you want but in return it leads to the risk to rise errors when accessing the value of a parameter or when calling the Parameters’ pretty_print method (which is very convenient for finding the parameter that is messed up).

This leads me to a second problem: when setting an incorrect expression, even with a syntax error that warns me that somethings got wrong with an exception, the expression is set to the parameter.

import lmfit

params = lmfit.Parameters()
params.add('a', 1.)
params.add('b', 2.)
params.add('c', 3.)

try:
    params['c'].expr = 'sin(b'  # syntax error
except Exception as e:
    print('Error1: ', e)
try:
    c = params['c'].value  # Despite the exception, the expression is set to the parameter
except Exception as e:
    print('Error2: ', e)

params['c'].expr = None
params['c'].expr = 'sin(d)'  # name error but don't rise an Exception
try:
    c = params['c'].value  # Name error is only raised by getting the value
except Exception as e:
    print('Error3: ', e)

params['c'].expr = 'unknown_function(a)'  # other kind of name error
try:
    c = params['c'].value  # Name error
except Exception as e:
    print('Error 4:', e)

I have looked into the the code and here is my proposal to solve both of this problem by small modifications of the Parameter class' __set_expression method.

def __set_expression(self, val):
    if val == '':
        val = None

    if not hasattr(self, '_expr_eval'):
        self._expr_eval = None
    if val is None:
        self._expr_ast = None
    if val is not None and self._expr_eval is not None:
        self._expr_eval.error = []
        self._expr_eval.error_msg = None
        expr_ast = self._expr_eval.parse(val)
        _ = self._expr_eval(expr_ast)  # This will leads to rise NameError if needed at the next line
        check_ast_errors(self._expr_eval)
        self._expr_ast = expr_ast  # Set to the parameter only after error check
        self._expr_deps = get_ast_names(self._expr_ast)
    if val is not None:
        self.vary = False
    self._expr = val

Marc

[Matt Newville]

On Tue, May 26, 2020 at 11:48 AM Marc Lenertz <lenert...@gmail.com> wrote:

Hello

I’m currently developing a graphical interface for changing my fit parameters and found some problems with the parameters’ expressions.

When I set an incorrect expression to a parameter only syntax errors will raise an exception.

Name errors will only occur once I access the parameter’s value. Is this a known behavior or a bug?

Yes, that is correct.  It is known behavior in the sense that it is the correct, expected behavior.

Syntax errors are definitely an error. Name errors are something that may be resolved. 

 

The only reason I see to keep this behavior is because you can add parameters that are linked together in the order you want

Yes, that is the reason.  

but in return it leads to the risk to rise errors when accessing the value of a parameter or when calling the Parameters’ pretty_print method (which is very convenient for finding the parameter that is messed up).

Well, syntax errors are not resolvable. Name errors are resolvable.  So we elect to delay complaining about name error until as late as possible.

 

This leads me to a second problem: when setting an incorrect expression, even with a syntax error that warns me that somethings got wrong with an exception, the expression is set to the parameter.

What do you think it should do?  Leaving it as the user set it means that the program/application/GUI can still retrieve the expression string and present "Hey that is a syntax error".   We could just replace "sin(b"  with "" or "that_is_a_syntax_error", but then the user would not be able to see what was wrong unless the calling program saved that expression in some other place. 

--
You received this message because you are subscribed to the Google Groups "lmfit-py" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lmfit-py+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/lmfit-py/90a4bade-17bc-492d-b0e6-c09c8e448e9e%40googlegroups.com.

--Matt 

[Marc Lenertz]

 Thanks Matt for the answers, 

Well, syntax errors are not resolvable. Name errors are resolvable.  So we elect to delay complaining about name error until as late as possible.

Ok, this maks sense, 

 

What do you think it should do?  Leaving it as the user set it means that the program/application/GUI can still retrieve the expression string and present "Hey that is a syntax error".   We could just replace "sin(b"  with "" or "that_is_a_syntax_error", but then the user would not be able to see what was wrong unless the calling program saved that expression in some other place. 

In my opinion the raise of the Exception is the warning  to help the user to identify the issue but you make a good point. What bothers me is that an incorrect expression leads to exceptions in the Parameters.pretty_print method, which are for me a good way to identify problems when working with a CLI. Even the representation of the object it self raises an exception if the expression is incorrect . Keeping your remarks in mind, I would suggest to handle the exceptions in the representation methods (Parameters.pretty_print, Parameters.__repr__, Parameter.__repr__, ...). Either intercept and raise a more explicit exception with an indication of the incriminated parameter or replace values that fails to be evaluated by an error flag. 

This second option also help to postpone NameError exceptions as late as possible and helps to identify problems in a more user friendly way. This could be handled through a Parameter.valuerepr method that returns a representation of the value or an error str if it fails and can be called by the representations methods  instead of _getval. It also keep the incriminated expression unchanged so that the user can see were the problem comes from. 

class Parameter:
    ....

    def valuerepr(self, precision=4, fmt='g', errflag="Error"):
        try:
            value = self._getval()
        except Exception:
            return errflag
        return '{v:.{p}{f}}'.format(v=value, p=precision, f=fmt)


    def __repr__(self):
        """Return printable representation of a Parameter object."""
        s = []
        sval = "value=%s" % repr(self.valuerepr())  # use 'valuerepr' instead of _getval
        if not self.vary and self._expr is None:
            sval += " (fixed)"
        elif self.stderr is not None:
            sval += " +/- %.3g" % self.stderr
        s.append(sval)
        s.append("bounds=[%s:%s]" % (repr(self.min), repr(self.max)))
        if self._expr is not None:
            s.append("expr='%s'" % self.expr)
        if self.brute_step is not None:
            s.append("brute_step=%s" % (self.brute_step))
        return "<Parameter '%s', %s>" % (self.name, ', '.join(s))

Something similar has to be applied for Parameters pretty_print method as well. If it's interesting I can write.

Other point, I said at the beginning I was developing a GUI for editing the fit parameters. What bothered me was that asteval was writing in the stderr channel instead of raising any Exception. I wanted to intercept such error before it appends because it leads to unexpected and uncontrolled text in my shell. This how I ended up digging more in details in the Parameters and Parameter classes code's. I finally succeed to solve this problem by setting my own asteval Interpreter instance to the Parameters object with a custom err_writer object. 

By the way, many many thanks for developing lmfit, it's realy a great package!

Marc 

Le mercredi 27 mai 2020 17:49:39 UTC+2, Matt Newville a écrit :