From b43f2242ad5978bc9cf236c948bc3bd3f584dee4 Mon Sep 17 00:00:00 2001 From: Peter Pinch Date: Mon, 7 May 2018 11:14:57 -0400 Subject: [PATCH] more descriptive error messages for UndefinedVariable --- common/lib/calc/calc/calc.py | 42 +++++++++++++++++-- common/lib/calc/calc/tests/test_calc.py | 4 +- common/lib/capa/capa/responsetypes.py | 8 ++-- .../lib/capa/capa/tests/test_responsetypes.py | 3 +- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/common/lib/calc/calc/calc.py b/common/lib/calc/calc/calc.py index 225edb4955..397e1bdc67 100644 --- a/common/lib/calc/calc/calc.py +++ b/common/lib/calc/calc/calc.py @@ -458,11 +458,45 @@ class ParseAugmenter(object): else: casify = lambda x: x.lower() # Lowercase for case insens. - # Test if casify(X) is valid, but return the actual bad input (i.e. X) bad_vars = set(var for var in self.variables_used if casify(var) not in valid_variables) - bad_vars.update(func for func in self.functions_used - if casify(func) not in valid_functions) if bad_vars: - raise UndefinedVariable(' '.join(sorted(bad_vars))) + varnames = ", ".join(sorted(bad_vars)) + message = "Invalid Input: {} not permitted in answer as a variable".format(varnames) + + # Check to see if there is a different case version of the variables + caselist = set() + if self.case_sensitive: + for var2 in bad_vars: + for var1 in valid_variables: + if var2.lower() == var1.lower(): + caselist.add(var1) + if len(caselist) > 0: + betternames = ', '.join(sorted(caselist)) + message += " (did you mean " + betternames + "?)" + + raise UndefinedVariable(message) + + bad_funcs = set(func for func in self.functions_used + if casify(func) not in valid_functions) + if bad_funcs: + funcnames = ', '.join(sorted(bad_funcs)) + message = "Invalid Input: {} not permitted in answer as a function".format(funcnames) + + # Check to see if there is a corresponding variable name + if any(casify(func) in valid_variables for func in bad_funcs): + message += " (did you forget to use * for multiplication?)" + + # Check to see if there is a different case version of the function + caselist = set() + if self.case_sensitive: + for func2 in bad_funcs: + for func1 in valid_functions: + if func2.lower() == func1.lower(): + caselist.add(func1) + if len(caselist) > 0: + betternames = ', '.join(sorted(caselist)) + message += " (did you mean " + betternames + "?)" + + raise UndefinedVariable(message) diff --git a/common/lib/calc/calc/tests/test_calc.py b/common/lib/calc/calc/tests/test_calc.py index 73f130049b..c5235c253c 100644 --- a/common/lib/calc/calc/tests/test_calc.py +++ b/common/lib/calc/calc/tests/test_calc.py @@ -541,8 +541,10 @@ class EvaluatorTest(unittest.TestCase): calc.evaluator({}, {}, "5+7*QWSEKO") with self.assertRaisesRegexp(calc.UndefinedVariable, 'r2'): calc.evaluator({'r1': 5}, {}, "r1+r2") - with self.assertRaisesRegexp(calc.UndefinedVariable, 'r1 r3'): + with self.assertRaisesRegexp(calc.UndefinedVariable, 'r1, r3'): calc.evaluator(variables, {}, "r1*r3", case_sensitive=True) + with self.assertRaisesRegexp(calc.UndefinedVariable, 'did you forget to use \*'): + calc.evaluator(variables, {}, "R1(R3 + 1)") def test_mismatched_parens(self): """ diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 4a5ce19692..cc549104d5 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1598,11 +1598,9 @@ class NumericalResponse(LoncapaResponse): # Catch a bunch of exceptions and give nicer messages to the student. try: student_float = evaluator({}, {}, student_answer) - except UndefinedVariable as undef_var: + except UndefinedVariable as err: raise StudentInputError( - _(u"You may not use variables ({bad_variables}) in numerical problems.").format( - bad_variables=text_type(undef_var), - ) + err.args[0] ) except UnmatchedParenthesis as err: raise StudentInputError( @@ -3110,7 +3108,7 @@ class FormulaResponse(LoncapaResponse): cgi.escape(answer) ) raise StudentInputError( - _("Invalid input: {bad_input} not permitted in answer.").format(bad_input=text_type(err)) + err.args[0] ) except UnmatchedParenthesis as err: log.debug( diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index b4779d6cf4..63c9b367ef 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -1625,7 +1625,8 @@ class NumericalResponseTest(ResponseTest): # pylint: disable=missing-docstring problem = self.build_problem(answer=4) errors = [ # (exception raised, message to student) - (calc.UndefinedVariable("x"), r"You may not use variables \(x\) in numerical problems"), + (calc.UndefinedVariable("Invalid Input: x not permitted in answer as a variable"), + r"Invalid Input: x not permitted in answer as a variable"), (ValueError("factorial() mess-up"), "Factorial function evaluated outside its domain"), (ValueError(), "Could not interpret '.*' as a number"), (pyparsing.ParseException("oopsie"), "Invalid math syntax"),