Merge pull request #126 from ChristopherChudzicki/convert-np-scalars

Convert np scalars

mitxgraders/helpers/calc/calc.py

@@ -33,7 +33,7 @@
 )
 from mitxgraders.exceptions import StudentFacingError
 from mitxgraders.helpers.validatorfuncs import get_number_of_args
-from mitxgraders.helpers.calc.math_array import MathArray
+from mitxgraders.helpers.calc.math_array import MathArray, is_vector
 from mitxgraders.helpers.calc.robust_pow import robust_pow
 from mitxgraders.helpers.calc.mathfuncs import (
     DEFAULT_VARIABLES, DEFAULT_FUNCTIONS, DEFAULT_SUFFIXES)
@@ -342,6 +342,45 @@ def evaluator(formula,
                        suffixes=math_interpreter.suffixes_used)
     return result, usage
 
+def cast_np_numeric_as_builtin(obj, map_across_lists=False):
+    """
+    Cast numpy numeric types as builtin python types.
+
+    NOTE: We do this because instances of np.number behave badly with MathArray.
+    See https://github.com/mitodl/mitx-grading-library/issues/124
+
+    Examples:
+    >>> import numpy as np
+    >>> x = 1.0
+    >>> x64 = np.float64(x)
+    >>> y = 5
+    >>> y64 = np.int64(y)
+    >>> z = 3 + 2j
+    >>> z128 = np.complex128(z)
+    >>> examples = [x, x64, y, y64, z, z128]
+    >>> [type(cast_np_numeric_as_builtin(example)) for example in examples]
+    [<type 'float'>, <type 'float'>, <type 'int'>, <type 'int'>, <type 'complex'>, <type 'complex'>]
+
+    Leaves MathArrays alone:
+    >>> from mitxgraders.helpers.calc.math_array import MathArray
+    >>> A = MathArray([1, 2, 3])
+    >>> cast_np_numeric_as_builtin(A)
+    MathArray([1, 2, 3])
+
+    Optionally, map across a list:
+    >>> target = [np.float64(1.0), np.float64(2.0)]
+    >>> result = cast_np_numeric_as_builtin(target, map_across_lists=True)
+    >>> [type(item) for item in result]
+    [<type 'float'>, <type 'float'>]
+
+    """
+    if isinstance(obj, np.number):
+        return np.asscalar(obj)
+    if map_across_lists and isinstance(obj, list):
+        return [np.asscalar(item) if isinstance(item, np.number) else item
+                for item in obj]
+    return obj
+
 class FormulaParser(object):
     """
     Parses a mathematical expression into a tree that can subsequently be evaluated
@@ -564,7 +603,7 @@ def handle_node(node):
             if not isinstance(node, ParseResults):
                 # Entry is either a (python) number or a string.
                 # Return it directly to the next level up.
-                return node
+                return cast_np_numeric_as_builtin(node)
 
             node_name = node.getName()
             if node_name not in self.actions:  # pragma: no cover
@@ -591,7 +630,7 @@ def handle_node(node):
             if any(np.any(np.isnan(r)) for r in as_list):
                 return float('nan')
 
-            return result
+            return cast_np_numeric_as_builtin(result, map_across_lists=True)
 
         # Find the value of the entire tree
         # Catch math errors that may arise
@@ -949,6 +988,9 @@ def eval_product(self, parse_result):
             parse_result: A list of numbers to combine, separated by "*" and "/"
             [a, "*", b, "/", c] = a*b/c
 
+        Has some extra logic to avoid ambiguous vector tirple products.
+        See https://github.com/mitodl/mitx-grading-library/issues/108
+
         Usage
         =====
         >>> parser = FormulaParser("1", {"%": 0.01})
@@ -958,23 +1000,33 @@ def eval_product(self, parse_result):
         Traceback (most recent call last):
         CalcError: Unexpected symbol + in eval_product
         """
-        num_vectors = sum([isinstance(operand, MathArray) and operand.ndim == 1
-                           for operand in parse_result])
-        if num_vectors >= 3:
-            raise CalcError("Multiplying three or more vectors is ambiguous. "
-                            "Please place parentheses around vector multiplications.")
+        double_vector_mult_has_occured = False
+        triple_vector_mult_error = CalcError(
+            "Multiplying three or more vectors is ambiguous. "
+            "Please place parentheses around vector multiplications."
+            )
 
         result = parse_result[0]
         data = parse_result[1:]
         while data:
             op = data.pop(0)
-            num = data.pop(0)
-            if op == '*':
-                result *= num
-            elif op == '/':
-                result /= num
+            value = data.pop(0)
+            if op == '/':
+                result /= value
+            elif op == '*':
+                if is_vector(value):
+                    if double_vector_mult_has_occured:
+                        raise triple_vector_mult_error
+                    elif is_vector(result):
+                        double_vector_mult_has_occured = True
+                result *= value
             else:
                 raise CalcError("Unexpected symbol {} in eval_product".format(op))
+
+            # Need to cast np numerics as builtins here (in addition to during
+            # handle_node) because the result is changing shape
+            result = cast_np_numeric_as_builtin(result)
+
         return result
 
     def eval_sum(self, parse_result):

mitxgraders/helpers/calc/exceptions.py

@@ -54,7 +54,7 @@ class ArgumentError(DomainError):
     """
     pass
 
-class MathArrayError(StudentFacingError):
+class MathArrayError(CalcError):
     """
     Thrown by MathArray when anticipated errors are made.
     """

mitxgraders/helpers/calc/math_array.py

@@ -68,6 +68,10 @@ def is_numberlike_zero_array(obj):
 def is_square(array):
     return array.ndim == 2 and array.shape[0] == array.shape[1]
 
+def is_vector(obj):
+    """Tests if obj is a vector MathArray"""
+    return isinstance(obj, MathArray) and obj.ndim == 1
+
 class MathArray(np.ndarray):
     """
     A modification of numpy's ndarray class that behaves more like a mathematician would expect.

tests/formulagrader/test_formulagrader.py

@@ -571,7 +571,7 @@ def test_fg_debug_log():
     "    'y': 3.5835764522666245,<br/>\n"
     "    'z': (1.875174422525385+2.7835460015641598j)}}<br/>\n"
     "Student Eval: (11.9397106851+2.78354600156j)<br/>\n"
-    "Compare to:  [(11.939710685061661+2.7835460015641598j)]<br/>\n"
+    "Compare to:  [(11.93971068506166+2.7835460015641598j)]<br/>\n"
     "Comparer Function: <function default_equality_comparer at 0x...><br/>\n"
     "Comparison Result: {{   'grade_decimal': 1.0, 'msg': '', 'ok': True}}<br/>\n"
     "</pre>"

tests/helpers/calc/test_calc_arrays.py

@@ -76,3 +76,33 @@ def test_triple_vector_product_raises_error():
 
     with raises(CalcError, match=match):
         evaluator("i*2*i*3*j", variables)[0]
+
+    # Next example should raise an operator shape error, not a triple vec error
+    match='Cannot divide by a vector'
+    with raises(CalcError, match=match):
+        evaluator("i*j/i*i*j", variables)[0]
+
+def test_matharray_errors_make_it_through():
+    """
+    There is some overlap between this test and the tests in test_math_array.
+
+    Main goal here is to make sure numpy numerics are not introduced during
+    evaluator(...) calls, because
+
+    np.float64(1.0) + MathArray([1, 2, 3])
+
+    does not throw an error.
+    """
+
+    v = MathArray([1, 2, 3])
+    variables = {
+        'v': v
+    }
+    with raises(CalcError, match="Cannot add/subtract"):
+        evaluator('v*v + v', variables)
+
+    with raises(CalcError, match="Cannot add/subtract"):
+        evaluator('v*v - v', variables)
+
+    with raises(CalcError, match="Cannot divide"):
+        evaluator('v*v/v', variables)