Merge pull request #50 from mitodl/no_case_insensitive

Removing case-insensitive input for FormulaGrader, IntegralGrader

docs/formula_grader.md

@@ -240,17 +240,7 @@ Tolerances are necessary because of numerical roundoff error that lead to small
 
 ## Case Sensitive Input
 
-By default, expressions are treated in a case sensitive manner. This means that variables `m` and `M` are distinct variables. If you want to grade answers in a case-insensitive manner, you can use the following.
-
-```python
-grader = FormulaGrader(
-    answers='2*sin(theta)*cos(theta)',
-    variables=['theta'],
-    case_sensitive=False
-)
-```
-
-This will allow students to use `SIN` and `COS` as well as `sin` and `cos`.
+All expressions are treated in a case sensitive manner. This means that variables `m` and `M` are distinct variables. Prior to version 1.1, we had a case-insensitive option available. However, this option is now deprecated, as it was causing issues in the codebase, nobody was using it (that we know of), and the majority of languages are case-sensitive anway.
 
 ## Suffixes
 

mitxgraders/formulagrader.py

@@ -27,19 +27,13 @@
     "CalcError"
 ]
 
-def casify(thestring, case_sensitive):
-    """Converts a string to lowercase if not case_sensitive"""
-    return thestring if case_sensitive else thestring.lower()
-
 # Some of these validators are useful to other classes, e.g., IntegralGrader
 def validate_blacklist_whitelist_config(blacklist, whitelist):
     """Validates the whitelist/blacklist configuration.
 
     Voluptuous should already have type-checked blacklist and whitelist. Now check:
     1. They are not both used
     2. All whitelist/blacklist functions actually exist
-
-    NOTE: whitelist/blacklist are not casified during configuration validation.
     """
     if blacklist and whitelist:
         raise ConfigError("Cannot whitelist and blacklist at the same time")
@@ -56,7 +50,7 @@ def validate_blacklist_whitelist_config(blacklist, whitelist):
         if func not in DEFAULT_FUNCTIONS:
             raise ConfigError("Unknown function in whitelist: {func}".format(func=func))
 
-def validate_forbidden_strings_not_used(expr, forbidden_strings, forbidden_msg, case_sensitive):
+def validate_forbidden_strings_not_used(expr, forbidden_strings, forbidden_msg):
     """
     Ignoring whitespace, checking that expr does not contain any forbidden_strings.
     Usage
@@ -65,54 +59,48 @@ def validate_forbidden_strings_not_used(expr, forbidden_strings, forbidden_msg,
     >>> validate_forbidden_strings_not_used(
     ... '2*sin(x)*cos(x)',
     ... ['*x', '+ x', '- x'],
-    ... 'A forbidden string was used!',
-    ... True
+    ... 'A forbidden string was used!'
     ... )
     True
 
     Fails validation if any forbidden string is used:
     >>> validate_forbidden_strings_not_used(
     ... 'sin(x+x)',
     ... ['*x', '+ x', '- x'],
-    ... 'A forbidden string was used!',
-    ... True
+    ... 'A forbidden string was used!'
     ... )
     Traceback (most recent call last):
     InvalidInput: A forbidden string was used!
     """
-    stripped_expr = casify(expr, case_sensitive).replace(' ', '')
+    stripped_expr = expr.replace(' ', '')
     for forbidden in forbidden_strings:
-        check_for = casify(forbidden, case_sensitive).replace(' ', '')
+        check_for = forbidden.replace(' ', '')
         if check_for in stripped_expr:
             # Don't give away the specific string that is being checked for!
             raise InvalidInput(forbidden_msg)
     return True
 
-def validate_only_permitted_functions_used(used_funcs, permitted_functions, case_sensitive):
+def validate_only_permitted_functions_used(used_funcs, permitted_functions):
     """
     Check that the used_funcs contains only permitted_functions
     Arguments:
         used_functions ({str}): set of used function names
         permitted_functions ({str}): set of permitted function names
-        case_sensitive (bool): whether comparison is case sensitive
     Usage
     =====
-    Case-sensitive comparison:
     >>> validate_only_permitted_functions_used(
     ... set(['f', 'sin']),
-    ... set(['f', 'g', 'sin', 'cos']),
-    ... True)
+    ... set(['f', 'g', 'sin', 'cos'])
+    ... )
     True
     >>> validate_only_permitted_functions_used(
     ... set(['f', 'Sin', 'h']),
-    ... set(['f', 'g', 'sin', 'cos']),
-    ... True)
+    ... set(['f', 'g', 'sin', 'cos'])
+    ... )
     Traceback (most recent call last):
     InvalidInput: Invalid Input: function(s) 'h', 'Sin' not permitted in answer
     """
-    casified_permitted = {casify(f, case_sensitive) for f in permitted_functions}
-    used_not_permitted = [f for f in used_funcs
-                          if casify(f, case_sensitive) not in casified_permitted]
+    used_not_permitted = [f for f in used_funcs if f not in permitted_functions]
     if used_not_permitted != []:
         func_names = ", ".join(["'{f}'".format(f=f) for f in used_not_permitted])
         message = "Invalid Input: function(s) {} not permitted in answer".format(func_names)
@@ -168,44 +156,35 @@ def get_permitted_functions(whitelist, blacklist, always_allowed):
         permitted_functions = set(always_allowed).union(whitelist)
     return permitted_functions
 
-def validate_required_functions_used(used_funcs, required_funcs, case_sensitive):
+def validate_required_functions_used(used_funcs, required_funcs):
     """
     Raise InvalidInput error if used_funcs does not contain all required_funcs
 
     Examples:
     >>> validate_required_functions_used(
     ... ['sin', 'cos', 'f', 'g'],
-    ... ['cos', 'f'],
-    ... True
+    ... ['cos', 'f']
     ... )
     True
     >>> validate_required_functions_used(
     ... ['sin', 'cos', 'F', 'g'],
-    ... ['cos', 'f'],
-    ... True
+    ... ['cos', 'f']
     ... )
     Traceback (most recent call last):
     InvalidInput: Invalid Input: Answer must contain the function f
-
-    Case insensitive:
-    >>> validate_required_functions_used(
-    ... ['sin', 'Cos', 'F', 'g'],
-    ... ['cos', 'f'],
-    ... False
-    ... )
-    True
     """
     for func in required_funcs:
-        func = casify(func, case_sensitive)
-        used_funcs = [casify(f, case_sensitive) for f in used_funcs]
+        used_funcs = [f for f in used_funcs]
         if func not in used_funcs:
             msg = "Invalid Input: Answer must contain the function {}"
             raise InvalidInput(msg.format(func))
     return True
 
 class FormulaGrader(ItemGrader):
     """
-    Grades mathematical expressions, like edX FormulaResponse.
+    Grades mathematical expressions, like edX FormulaResponse. Note that comparison will
+    always be performed in a case-sensitive nature, unlike edX, which allows for a
+    case-insensitive comparison.
 
     Configuration options:
         user_functions (dict): A dictionary of user-defined functions that students can
@@ -243,9 +222,6 @@ class FormulaGrader(ItemGrader):
             the solutions. Can be expressed as an absolute number (eg, 0.1), or as a string
             percentage (default '0.1%'). Must be positive or zero.
 
-        case_sensitive (bool): Should comparison of variable and function names be performed
-            in a case sensitive manner? (default True)
-
         metric_suffixes (bool): Should metric affixes be available to students to modify
             answers (default False). If true, then "2m" == 0.002, for example.
 
@@ -292,7 +268,6 @@ def schema_config(self):
             Required('forbidden_message', default=forbidden_default): str,
             Required('required_functions', default=[]): [str],
             Required('tolerance', default='0.01%'): Any(PercentageString, NonNegative(Number)),
-            Required('case_sensitive', default=True): bool,
             Required('metric_suffixes', default=False): bool,
             Required('samples', default=5): Positive(int),
             Required('variables', default=[]): [str],
@@ -464,15 +439,13 @@ def raw_check(self, answer, student_input):
             # Compute expressions
             comparer_params_eval = [
                 evaluator(formula=param,
-                          case_sensitive=self.config['case_sensitive'],
                           variables=varlist,
                           functions=funclist,
                           suffixes=self.suffixes)[0]
                 for param in answer['expect']['comparer_params']
                 ]
 
             student_eval, used_funcs = evaluator(student_input,
-                                                 case_sensitive=self.config['case_sensitive'],
                                                  variables=varlist,
                                                  functions=funclist,
                                                  suffixes=self.suffixes)
@@ -521,16 +494,13 @@ def log_sample_info(self, index, varlist, funclist, student_eval,
 
     def post_eval_validation(self, expr, used_funcs):
         """Runs several post-evaluation validator functions"""
-        case_sensitive = self.config['case_sensitive']
         validate_forbidden_strings_not_used(expr,
                                             self.config['forbidden_strings'],
-                                            self.config['forbidden_message'],
-                                            case_sensitive)
+                                            self.config['forbidden_message'])
 
-        validate_required_functions_used(used_funcs, self.config['required_functions'],
-                                         case_sensitive)
+        validate_required_functions_used(used_funcs, self.config['required_functions'])
 
-        validate_only_permitted_functions_used(used_funcs, self.permitted_functions, case_sensitive)
+        validate_only_permitted_functions_used(used_funcs, self.permitted_functions)
 
 class NumericalGrader(FormulaGrader):
     """

mitxgraders/helpers/calc.py

@@ -218,7 +218,7 @@ def __init__(self):
         """Initializes the cache"""
         self.cache = {}
 
-    def get_parser(self, formula, case_sensitive, suffixes):
+    def get_parser(self, formula, suffixes):
         """Get a ParseAugmenter object for a given formula"""
         # Check the formula for matching parentheses
         count = 0
@@ -246,15 +246,15 @@ def get_parser(self, formula, case_sensitive, suffixes):
         suffixstr = ""
         for key in suffixes:
             suffixstr += key
-        key = (stripformula, case_sensitive, ''.join(sorted(suffixstr)))
+        key = (stripformula, ''.join(sorted(suffixstr)))
 
         # Check if it's in the cache
         parser = self.cache.get(key, None)
         if parser is not None:
             return parser
 
         # It's not, so construct it
-        parser = ParseAugmenter(stripformula, case_sensitive, suffixes)
+        parser = ParseAugmenter(stripformula, suffixes)
         try:
             parser.parse_algebra()
         except ParseException:
@@ -270,23 +270,24 @@ def get_parser(self, formula, case_sensitive, suffixes):
 parsercache = ParserCache()
 
 
-def evaluator(formula, variables, functions, suffixes, case_sensitive=True):
+def evaluator(formula, variables, functions, suffixes):
     """
     Evaluate an expression; that is, take a string of math and return a float.
 
     -Variables are passed as a dictionary from string to value. They must be
      python numbers.
     -Unary functions are passed as a dictionary from string to function.
+    -Everything is case sensitive (note, this is different to edX!)
 
     Usage
     =====
-    >>> evaluator("1+1", {}, {}, {}, True)
+    >>> evaluator("1+1", {}, {}, {})
     (2.0, set([]))
-    >>> evaluator("1+x", {"x": 5}, {}, {}, True)
+    >>> evaluator("1+x", {"x": 5}, {}, {})
     (6.0, set([]))
-    >>> evaluator("square(2)", {}, {"square": lambda x: x*x}, {}, True)
+    >>> evaluator("square(2)", {}, {"square": lambda x: x*x}, {})
     (4.0, set(['square']))
-    >>> evaluator("", {}, {}, {}, True)
+    >>> evaluator("", {}, {}, {})
     (nan, set([]))
     """
     if formula is None:
@@ -298,23 +299,11 @@ def evaluator(formula, variables, functions, suffixes, case_sensitive=True):
         return float('nan'), set()
 
     # Parse the tree
-    math_interpreter = parsercache.get_parser(formula, case_sensitive, suffixes)
-
-    # If we're not case sensitive, then lower all variables and functions
-    if not case_sensitive:
-        # Also make the variables and functions lower case
-        variables = {key.lower(): value for key, value in variables.iteritems()}
-        functions = {key.lower(): value for key, value in functions.iteritems()}
+    math_interpreter = parsercache.get_parser(formula, suffixes)
 
     # Check the variables and functions
     math_interpreter.check_variables(variables, functions)
 
-    # Some helper functions...
-    if case_sensitive:
-        casify = lambda x: x
-    else:
-        casify = lambda x: x.lower()  # Lowercase for case insensitive
-
     def eval_number(parse_result):
         """
         Create a float out of string parts
@@ -331,7 +320,7 @@ def eval_number(parse_result):
     def eval_function(parse_result):
         name, arg = parse_result
         try:
-            return functions[casify(name)](arg)
+            return functions[name](arg)
         except ZeroDivisionError:
             # It would be really nice to tell student the symbolic argument as part of this message,
             # but making symbolic argument available would require some nontrivial restructing
@@ -360,7 +349,7 @@ def eval_function(parse_result):
     # Create a recursion to evaluate the tree
     evaluate_actions = {
         'number': eval_number,
-        'variable': lambda x: variables[casify(x[0])],
+        'variable': lambda x: variables[x[0]],
         'function': eval_function,
         'atom': eval_atom,
         'power': eval_power,
@@ -385,17 +374,15 @@ class ParseAugmenter(object):
     """
     Holds the data for a particular parse.
 
-    Retains the `math_expr` and `case_sensitive` so they needn't be passed
-    around method to method.
+    Retains `math_expr` so it needn't be passed around method to method.
     Eventually holds the parse tree and sets of variables as well.
     """
-    def __init__(self, math_expr, case_sensitive, suffixes):
+    def __init__(self, math_expr, suffixes):
         """
         Create the ParseAugmenter for a given math expression string.
 
         Do the parsing later, when called like `OBJ.parse_algebra()`.
         """
-        self.case_sensitive = case_sensitive
         self.math_expr = math_expr
         self.tree = None
         self.variables_used = set()
@@ -542,38 +529,43 @@ def check_variables(self, valid_variables, valid_functions):
         Confirm that all the variables and functions used in the tree are defined.
         Otherwise, raise an UndefinedVariable or UndefinedFunction
         """
-        if self.case_sensitive:
-            casify = lambda x: x
-        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)
+                       if var not in valid_variables)
         if bad_vars:
             message = "Invalid Input: {} not permitted in answer as a variable"
             varnames = ", ".join(sorted(bad_vars))
+
+            # Check to see if there is a different case version of the variable
+            caselist = set()
+            for var2 in bad_vars:
+                for var1 in valid_variables:
+                    if var1.lower() == var2.lower():
+                        caselist.add(var1)
+            if len(caselist) > 0:
+                betternames = ', '.join(sorted(caselist))
+                message += " (did you mean " + betternames + "?)"
+
             raise UndefinedVariable(message.format(varnames))
 
         bad_funcs = set(func for func in self.functions_used
-                        if casify(func) not in valid_functions)
+                        if func not in valid_functions)
         if bad_funcs:
             funcnames = ', '.join(sorted(bad_funcs))
             message = "Invalid Input: {} not permitted in answer as a function"
 
             # Check to see if there is a corresponding variable name
-            if any(casify(func) in valid_variables for func in bad_funcs):
+            if any(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 + "?)"
+            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 UndefinedFunction(message.format(funcnames))