Address misc. comments from PR

common/lib/calc/calc.py

@@ -60,10 +60,10 @@ DEFAULT_VARIABLES = {
     'j': numpy.complex(0, 1),
     'e': numpy.e,
     'pi': numpy.pi,
-    'k': scipy.constants.k,
-    'c': scipy.constants.c,
-    'T': 298.15,
-    'q': scipy.constants.e
+    'k': scipy.constants.k,  # Boltzmann: 1.3806488e-23 (Joules/Kelvin)
+    'c': scipy.constants.c,  # Light Speed: 2.998e8 (m/s)
+    'T': 298.15,  # 0 deg C = T Kelvin
+    'q': scipy.constants.e  # Fund. Charge: 1.602176565e-19 (Coulombs)
 }
 
 # We eliminated the following extreme suffixes:
@@ -126,8 +126,9 @@ def eval_atom(parse_result):
 
     In the case of parenthesis, ignore them.
     """
-    float_children = [k for k in parse_result if isinstance(k, numbers.Number)]
-    return float_children[0]
+    # Find first number in the list
+    result = next(k for k in parse_result if isinstance(k, numbers.Number))
+    return result
 
 
 def eval_power(parse_result):

common/lib/calc/preview.py

@@ -158,7 +158,7 @@ def function_closure(functions, casify):
         """
         fname = children[0].latex
         if casify(fname) not in functions:
-            pass
+            pass  # TODO turn unknown function red or give some kind of error
 
         # Wrap the input of the function with parens or braces.
         inner = children[1].latex

common/lib/calc/tests/test_calc.py

@@ -50,10 +50,7 @@ class EvaluatorTest(unittest.TestCase):
         """
         Test that things like '4.' will be 4 and not throw an error
         """
-        try:
-            self.assertEqual(4.0, calc.evaluator({}, {}, '4.'))
-        except ParseException:  # pragma: no cover
-            self.fail("'4.' is a valid input, but threw an exception")
+        self.assertEqual(4.0, calc.evaluator({}, {}, '4.'))
 
     def test_exponential_answer(self):
         """
@@ -362,17 +359,14 @@ class EvaluatorTest(unittest.TestCase):
 
         # Of the form ('expr', python value, tolerance (or None for exact))
         default_variables = [
+            ('i', 1j, None),
             ('j', 1j, None),
-            ('e', 2.7183, 1e-3),
-            ('pi', 3.1416, 1e-3),
-            # c = speed of light
-            ('c', 2.998e8, 1e5),
-            # 0 deg C = T Kelvin
-            ('T', 298.15, 0.01),
-            # Note k = scipy.constants.k = 1.3806488e-23
-            ('k', 1.3806488e-23, 1e-26),
-            # Note q = scipy.constants.e = 1.602176565e-19
-            ('q', 1.602176565e-19, 1e-22)
+            ('e', 2.7183, 1e-4),
+            ('pi', 3.1416, 1e-4),
+            ('k', 1.3806488e-23, 1e-26),  # Boltzmann constant (Joules/Kelvin)
+            ('c', 2.998e8, 1e5),  # Light Speed in (m/s)
+            ('T', 298.15, 0.01),  # 0 deg C = T Kelvin
+            ('q', 1.602176565e-19, 1e-22)  # Fund. Charge (Coulombs)
         ]
         for (variable, value, tolerance) in default_variables:
             fail_msg = "Failed on constant '{0}', not within bounds".format(
@@ -382,8 +376,10 @@ class EvaluatorTest(unittest.TestCase):
             if tolerance is None:
                 self.assertEqual(value, result, msg=fail_msg)
             else:
-                self.assertAlmostEqual(value, result,
-                                       delta=tolerance, msg=fail_msg)
+                self.assertAlmostEqual(
+                    value, result,
+                    delta=tolerance, msg=fail_msg
+                )
 
     def test_complex_expression(self):
         """
@@ -514,20 +510,35 @@ class EvaluatorTest(unittest.TestCase):
             -1, delta=1e-3
         )
 
-    def test_function_case_sensitivity(self):
+    def test_function_case_insensitive(self):
         """
-        Test the case sensitivity of functions
+        Test case insensitive evaluation
+
+        Normal functions with some capitals should be fine
         """
-        functions = {'f': lambda x: x, 'F': lambda x: x + 1}
+        self.assertAlmostEqual(
+            -0.28,
+            calc.evaluator({}, {}, 'SiN(6)', case_sensitive=False),
+            delta=1e-3
+        )
+
+    def test_function_case_sensitive(self):
+        """
+        Test case sensitive evaluation
 
-        # Test case insensitive evaluation
-        # Both evaulations should call the same function
-        self.assertEqual(calc.evaluator({}, functions, 'f(6)'),
-                         calc.evaluator({}, functions, 'F(6)'))
-        # Test case sensitive evaluation
-        self.assertNotEqual(
-            calc.evaluator({}, functions, 'f(6)', case_sensitive=True),
-            calc.evaluator({}, functions, 'F(6)', case_sensitive=True)
+        Incorrectly capitilized should fail
+        Also, it should pick the correct version of a function.
+        """
+        with self.assertRaisesRegexp(calc.UndefinedVariable, 'SiN'):
+            calc.evaluator({}, {}, 'SiN(6)', case_sensitive=True)
+
+        # With case sensitive turned on, it should pick the right function
+        functions = {'f': lambda x: x, 'F': lambda x: x + 1}
+        self.assertEqual(
+            calc.evaluator({}, functions, 'f(6)', case_sensitive=True), 6
+        )
+        self.assertEqual(
+            calc.evaluator({}, functions, 'F(6)', case_sensitive=True), 7
         )
 
     def test_undefined_vars(self):

common/lib/capa/capa/inputtypes.py

@@ -1040,15 +1040,16 @@ class ChemicalEquationInput(InputTypeBase):
 
         result = {'preview': '',
                   'error': ''}
-        formula = data['formula']
-        if formula is None:
+        try:
+            formula = data['formula']
+        except KeyError:
             result['error'] = "No formula specified."
             return result
 
         try:
             result['preview'] = chemcalc.render_to_html(formula)
         except pyparsing.ParseException as p:
-            result['error'] = "Couldn't parse formula: {0}".format(p)
+            result['error'] = u"Couldn't parse formula: {0}".format(p.msg)
         except Exception:
             # this is unexpected, so log
             log.warning(
@@ -1100,12 +1101,12 @@ class FormulaEquationInput(InputTypeBase):
 
     def preview_formcalc(self, get):
         """
-        Render an html preview of a formula formula or equation.  get should
-        contain a key 'formula' and value 'some formula string'.
+        Render an preview of a formula or equation. `get` should
+        contain a key 'formula' with a math expression.
 
         Returns a json dictionary:
         {
-           'preview' : 'the-preview-html' or ''
+           'preview' : '<some latex>' or ''
            'error' : 'the-error' or ''
            'request_start' : <time sent with request>
         }
@@ -1113,17 +1114,23 @@ class FormulaEquationInput(InputTypeBase):
 
         result = {'preview': '',
                   'error': ''}
-        formula = get['formula']
-        if formula is None:
+
+        try:
+            formula = get['formula']
+        except KeyError:
             result['error'] = "No formula specified."
             return result
 
-        result['request_start'] = int(get['request_start'])
+        result['request_start'] = int(get.get('request_start', 0))
 
         try:
-            result['preview'] = latex_preview(formula)  # TODO add references to variables, etc&
+            # TODO add references to valid variables and functions
+            # At some point, we might want to mark invalid variables as red
+            # or something, and this is where we would need to pass those in.
+            result['preview'] = latex_preview(formula)
         except pyparsing.ParseException as err:
-            result['error'] = u"Couldn't parse formula: {0}".format(err.message)
+            result['error'] = u"Couldn't parse formula: {}".format(err.msg)
+            result['formula'] = formula
         except Exception:
             # this is unexpected, so log
             log.warning(

common/lib/capa/capa/tests/test_inputtypes.py

@@ -729,10 +729,57 @@ class ChemicalEquationTest(unittest.TestCase):
         data = {'formula': "H"}
         response = self.the_input.handle_ajax("preview_chemcalc", data)
 
-        self.assertTrue('preview' in response)
+        self.assertIn('preview', response)
         self.assertNotEqual(response['preview'], '')
         self.assertEqual(response['error'], "")
 
+    def test_ajax_bad_method(self):
+        """
+        With a bad dispatch, we shouldn't recieve anything
+        """
+        response = self.the_input.handle_ajax("obviously_not_real", {})
+        self.assertEqual(response, {})
+
+    def test_ajax_no_formula(self):
+        """
+        When we ask for a formula rendering, there should be an error if no formula
+        """
+        response = self.the_input.handle_ajax("preview_chemcalc", {})
+        self.assertIn('error', response)
+        self.assertEqual(response['error'], "No formula specified.")
+
+    def test_ajax_parse_err(self):
+        """
+        With parse errors, ChemicalEquationInput should give an error message
+        """
+        # Simulate answering a problem that raises the exception
+        with patch('capa.inputtypes.chemcalc.render_to_html') as mock_render:
+            mock_render.side_effect = ParseException(u"ȧƈƈḗƞŧḗḓ ŧḗẋŧ ƒǿř ŧḗşŧīƞɠ")
+            response = self.the_input.handle_ajax(
+                "preview_chemcalc",
+                {'formula': 'H2O + invalid chemistry'}
+            )
+
+        self.assertIn('error', response)
+        self.assertTrue("Couldn't parse formula" in response['error'])
+
+    @patch('capa.inputtypes.log')
+    def test_ajax_other_err(self, mock_log):
+        """
+        With other errors, test that ChemicalEquationInput also logs it
+        """
+        with patch('capa.inputtypes.chemcalc.render_to_html') as mock_render:
+            mock_render.side_effect = Exception()
+            response = self.the_input.handle_ajax(
+                "preview_chemcalc",
+                {'formula': 'H2O + superterrible chemistry'}
+            )
+        mock_log.warning.assert_called_once_with(
+            "Error while previewing chemical formula", exc_info=True
+        )
+        self.assertIn('error', response)
+        self.assertEqual(response['error'], "Error while rendering preview")
+
 
 class FormulaEquationTest(unittest.TestCase):
     """
@@ -744,7 +791,7 @@ class FormulaEquationTest(unittest.TestCase):
 
         element = etree.fromstring(xml_str)
 
-        state = {'value': 'x^2+1/2', }
+        state = {'value': 'x^2+1/2'}
         self.the_input = lookup_tag('formulaequationinput')(test_system(), element, state)
 
     def test_rendering(self):
@@ -769,7 +816,7 @@ class FormulaEquationTest(unittest.TestCase):
         data = {'formula': "x^2+1/2", 'request_start': 0}
         response = self.the_input.handle_ajax("preview_formcalc", data)
 
-        self.assertTrue('preview' in response)
+        self.assertIn('preview', response)
         self.assertNotEqual(response['preview'], '')
         self.assertEqual(response['error'], "")
         self.assertEqual(response['request_start'], data['request_start'])
@@ -787,9 +834,9 @@ class FormulaEquationTest(unittest.TestCase):
         """
         response = self.the_input.handle_ajax(
             "preview_formcalc",
-            {'formula': None, 'request_start': 1, }
+            {'request_start': 1, }
         )
-        self.assertTrue('error' in response)
+        self.assertIn('error', response)
         self.assertEqual(response['error'], "No formula specified.")
 
     def test_ajax_parse_err(self):
@@ -804,8 +851,9 @@ class FormulaEquationTest(unittest.TestCase):
                 {'formula': 'x^2+1/2', 'request_start': 1, }
             )
 
-        self.assertTrue('error' in response)
-        self.assertTrue("Couldn't parse formula" in response['error'])
+        self.assertIn('error', response)
+        self.assertEqual(response['error'],
+                         u"Couldn't parse formula: ȧƈƈḗƞŧḗḓ ŧḗẋŧ ƒǿř ŧḗşŧīƞɠ")
 
     @patch('capa.inputtypes.log')
     def test_ajax_other_err(self, mock_log):
@@ -821,7 +869,7 @@ class FormulaEquationTest(unittest.TestCase):
         mock_log.warning.assert_called_once_with(
             "Error while previewing formula", exc_info=True
         )
-        self.assertTrue('error' in response)
+        self.assertIn('error', response)
         self.assertEqual(response['error'], "Error while rendering preview")