Merge pull request #841 from MITx/victor/capa_cleanup

Victor/capa cleanup

Looks good.  Yes, the code exec part will need more thought, particularly with respect to security.

common/lib/capa/capa/calc.py

@@ -48,7 +48,7 @@ general_whitespace = re.compile('[^\w]+')
 
 
 def check_variables(string, variables):
-    '''  Confirm the only variables in string are defined.
+    '''Confirm the only variables in string are defined.
 
     Pyparsing uses a left-to-right parser, which makes the more
     elegant approach pretty hopeless.
@@ -56,7 +56,8 @@ def check_variables(string, variables):
     achar = reduce(lambda a,b:a|b ,map(Literal,alphas)) # Any alphabetic character
     undefined_variable = achar + Word(alphanums)
     undefined_variable.setParseAction(lambda x:UndefinedVariable("".join(x)).raiseself())
-    varnames = varnames | undefined_variable'''
+    varnames = varnames | undefined_variable
+    '''
     possible_variables = re.split(general_whitespace, string)  # List of all alnums in string
     bad_variables = list()
     for v in possible_variables:
@@ -71,7 +72,8 @@ def check_variables(string, variables):
 
 
 def evaluator(variables, functions, string, cs=False):
-    ''' Evaluate an expression. Variables are passed as a dictionary
+    '''
+    Evaluate an expression. Variables are passed as a dictionary
     from string to value. Unary functions are passed as a dictionary
     from string to function. Variables must be floats.
     cs: Case sensitive
@@ -108,6 +110,7 @@ def evaluator(variables, functions, string, cs=False):
 
     if string.strip() == "":
         return float('nan')
+
     ops = {"^": operator.pow,
             "*": operator.mul,
             "/": operator.truediv,
@@ -169,14 +172,19 @@ def evaluator(variables, functions, string, cs=False):
     def func_parse_action(x):
         return [all_functions[x[0]](x[1])]
 
-    number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch())  # SI suffixes and percent
+    # SI suffixes and percent
+    number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch())
     (dot, minus, plus, times, div, lpar, rpar, exp) = map(Literal, ".-+*/()^")
 
     number_part = Word(nums)
-    inner_number = (number_part + Optional("." + number_part)) | ("." + number_part)  # 0.33 or 7 or .34
-    number = Optional(minus | plus) + inner_number + \
-        Optional(CaselessLiteral("E") + Optional("-") + number_part) + \
-        Optional(number_suffix)  # 0.33k or -17
+
+    # 0.33 or 7 or .34
+    inner_number = (number_part + Optional("." + number_part)) | ("." + number_part)
+
+    # 0.33k or -17
+    number = (Optional(minus | plus) + inner_number
+              + Optional(CaselessLiteral("E") + Optional("-") + number_part)
+              + Optional(number_suffix))
     number = number.setParseAction(number_parse_action)  # Convert to number
 
     # Predefine recursive variables
@@ -201,9 +209,11 @@ def evaluator(variables, functions, string, cs=False):
         varnames.setParseAction(lambda x: map(lambda y: all_variables[y], x))
     else:
         varnames = NoMatch()
+
     # Same thing for functions.
     if len(all_functions) > 0:
-        funcnames = sreduce(lambda x, y: x | y, map(lambda x: CasedLiteral(x), all_functions.keys()))
+        funcnames = sreduce(lambda x, y: x | y,
+                            map(lambda x: CasedLiteral(x), all_functions.keys()))
         function = funcnames + lpar.suppress() + expr + rpar.suppress()
         function.setParseAction(func_parse_action)
     else:

common/lib/capa/capa/correctmap.py

@@ -5,23 +5,26 @@
 
 
 class CorrectMap(object):
-    '''
+    """
     Stores map between answer_id and response evaluation result for each question
     in a capa problem.  The response evaluation result for each answer_id includes
     (correctness, npoints, msg, hint, hintmode).
 
     - correctness : either 'correct' or 'incorrect'
     - npoints     : None, or integer specifying number of points awarded for this answer_id
-    - msg         : string (may have HTML) giving extra message response (displayed below textline or textbox)
-    - hint        : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg)
+    - msg         : string (may have HTML) giving extra message response
+                    (displayed below textline or textbox)
+    - hint        : string (may have HTML) giving optional hint
+                    (displayed below textline or textbox, above msg)
     - hintmode    : one of (None,'on_request','always') criteria for displaying hint
     - queuestate  : Dict {key:'', time:''} where key is a secret string, and time is a string dump
                     of a DateTime object in the format '%Y%m%d%H%M%S'. Is None when not queued
 
     Behaves as a dict.
-    '''
+    """
     def __init__(self, *args, **kwargs):
-        self.cmap = dict()		# start with empty dict
+        # start with empty dict
+        self.cmap = dict()
         self.items = self.cmap.items
         self.keys = self.cmap.keys
         self.set(*args, **kwargs)
@@ -33,7 +36,15 @@ class CorrectMap(object):
         return self.cmap.__iter__()
 
     # See the documentation for 'set_dict' for the use of kwargs
-    def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None, **kwargs):
+    def set(self,
+            answer_id=None,
+            correctness=None,
+            npoints=None,
+            msg='',
+            hint='',
+            hintmode=None,
+            queuestate=None, **kwargs):
+
         if answer_id is not None:
             self.cmap[answer_id] = {'correctness': correctness,
                                     'npoints': npoints,
@@ -56,12 +67,13 @@ class CorrectMap(object):
         '''
         Set internal dict of CorrectMap to provided correct_map dict
 
-        correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This means that
-        when the definition of CorrectMap (e.g. its properties) are altered, existing correct_map dict
-        not coincide with the newest CorrectMap format as defined by self.set.
+        correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This
+        means that when the definition of CorrectMap (e.g. its properties) are altered,
+        an existing correct_map dict not coincide with the newest CorrectMap format as
+        defined by self.set.
 
         For graceful migration, feed the contents of each correct map to self.set, rather than
-        making a direct copy of the given correct_map dict. This way, the common keys between 
+        making a direct copy of the given correct_map dict. This way, the common keys between
         the incoming correct_map dict and the new CorrectMap instance will be written, while
         mismatched keys will be gracefully ignored.
 
@@ -69,14 +81,20 @@ class CorrectMap(object):
             If correct_map is a one-level dict, then convert it to the new dict of dicts format.
         '''
         if correct_map and not (type(correct_map[correct_map.keys()[0]]) == dict):
-            self.__init__()	                                  # empty current dict
-            for k in correct_map: self.set(k, correct_map[k]) # create new dict entries
+            # empty current dict
+            self.__init__()
+
+            # create new dict entries
+            for k in correct_map:
+                self.set(k, correct_map[k])
         else:
             self.__init__()
-            for k in correct_map: self.set(k, **correct_map[k])
+            for k in correct_map:
+                self.set(k, **correct_map[k])
 
     def is_correct(self, answer_id):
-        if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct'
+        if answer_id in self.cmap:
+            return self.cmap[answer_id]['correctness'] == 'correct'
         return None
 
     def is_queued(self, answer_id):
@@ -94,14 +112,18 @@ class CorrectMap(object):
             return npoints
         elif self.is_correct(answer_id):
             return 1
-        return 0 # if not correct and no points have been assigned, return 0
+         # if not correct and no points have been assigned, return 0
+        return 0
 
     def set_property(self, answer_id, property, value):
-        if answer_id in self.cmap: self.cmap[answer_id][property] = value
-        else: self.cmap[answer_id] = {property: value}
+        if answer_id in self.cmap:
+            self.cmap[answer_id][property] = value
+        else:
+            self.cmap[answer_id] = {property: value}
 
     def get_property(self, answer_id, property, default=None):
-        if answer_id in self.cmap: return self.cmap[answer_id].get(property, default)
+        if answer_id in self.cmap:
+            return self.cmap[answer_id].get(property, default)
         return default
 
     def get_correctness(self, answer_id):

common/lib/capa/capa/eia.py

-""" Standard resistor codes.
+"""
+Standard resistor codes.
 http://en.wikipedia.org/wiki/Electronic_color_code
 """
 E6 = [10, 15, 22, 33, 47, 68]
+
 E12 = [10, 12, 15, 18, 22, 27, 33, 39, 47, 56, 68, 82]
+
 E24 = [10, 12, 15, 18, 22, 27, 33, 39, 47, 56, 68, 82, 11, 13, 16, 20, 24, 30, 36, 43, 51, 62, 75, 91]
+
 E48 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953]
+
 E96 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 102, 124, 150, 182, 221, 267, 324, 392, 475, 576, 698, 845, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 107, 130, 158, 191, 232, 280, 340, 412, 499, 604, 732, 887, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 113, 137, 165, 200, 243, 294, 357, 432, 523, 634, 768, 931, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953, 118, 143, 174, 210, 255, 309, 374, 453, 549, 665, 806, 976]
+
 E192 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 101, 123, 149, 180, 218, 264, 320, 388, 470, 569, 690, 835, 102, 124, 150, 182, 221, 267, 324, 392, 475, 576, 698, 845, 104, 126, 152, 184, 223, 271, 328, 397, 481, 583, 706, 856, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 106, 129, 156, 189, 229, 277, 336, 407, 493, 597, 723, 876, 107, 130, 158, 191, 232, 280, 340, 412, 499, 604, 732, 887, 109, 132, 160, 193, 234, 284, 344, 417, 505, 612, 741, 898, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 111, 135, 164, 198, 240, 291, 352, 427, 517, 626, 759, 920, 113, 137, 165, 200, 243, 294, 357, 432, 523, 634, 768, 931, 114, 138, 167, 203, 246, 298, 361, 437, 530, 642, 777, 942, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953, 117, 142, 172, 208, 252, 305, 370, 448, 542, 657, 796, 965, 118, 143, 174, 210, 255, 309, 374, 453, 549, 665, 806, 976, 120, 145, 176, 213, 258, 312, 379, 459, 556, 673, 816, 988]

common/lib/capa/capa/util.py

@@ -53,8 +53,4 @@ def is_file(file_to_test):
     '''
     Duck typing to check if 'file_to_test' is a File object
     '''
-    is_file = True
-    for method in ['read', 'name']:
-        if not hasattr(file_to_test, method):
-            is_file = False
-    return is_file 
+    return all(hasattr(file_to_test, method) for method in ['read', 'name'])

common/lib/capa/capa/xqueue_interface.py

@@ -12,7 +12,7 @@ dateformat = '%Y%m%d%H%M%S'
 
 def make_hashkey(seed):
     '''
-    Generate a string key by hashing 
+    Generate a string key by hashing
     '''
     h = hashlib.md5()
     h.update(str(seed))
@@ -20,27 +20,27 @@ def make_hashkey(seed):
 
 
 def make_xheader(lms_callback_url, lms_key, queue_name):
-    '''
+    """
     Generate header for delivery and reply of queue request.
 
     Xqueue header is a JSON-serialized dict:
         { 'lms_callback_url': url to which xqueue will return the request (string),
-          'lms_key': secret key used by LMS to protect its state (string), 
+          'lms_key': secret key used by LMS to protect its state (string),
           'queue_name': designate a specific queue within xqueue server, e.g. 'MITx-6.00x' (string)
         }
-    '''
+    """
     return json.dumps({ 'lms_callback_url': lms_callback_url,
                         'lms_key': lms_key,
                         'queue_name': queue_name })
 
 
 def parse_xreply(xreply):
-    '''
+    """
     Parse the reply from xqueue. Messages are JSON-serialized dict:
         { 'return_code': 0 (success), 1 (fail)
           'content': Message from xqueue (string)
         }
-    '''
+    """
     try:
         xreply = json.loads(xreply)
     except ValueError, err:
@@ -61,11 +61,11 @@ class XQueueInterface(object):
         self.url  = url
         self.auth = django_auth
         self.session = requests.session(auth=requests_auth)
-        
+
     def send_to_queue(self, header, body, files_to_upload=None):
-        '''
+        """
         Submit a request to xqueue.
-        
+
         header: JSON-serialized dict in the format described in 'xqueue_interface.make_xheader'
 
         body: Serialized data for the receipient behind the queueing service. The operation of
@@ -74,14 +74,16 @@ class XQueueInterface(object):
         files_to_upload: List of file objects to be uploaded to xqueue along with queue request
 
         Returns (error_code, msg) where error_code != 0 indicates an error
-        '''
+        """
         # Attempt to send to queue
         (error, msg) = self._send_to_queue(header, body, files_to_upload)
 
-        if error and (msg == 'login_required'): # Log in, then try again
+        # Log in, then try again
+        if error and (msg == 'login_required'):
             self._login()
             if files_to_upload is not None:
-                for f in files_to_upload: # Need to rewind file pointers
+                # Need to rewind file pointers
+                for f in files_to_upload:
                     f.seek(0)
             (error, msg) = self._send_to_queue(header, body, files_to_upload)
 
@@ -91,18 +93,18 @@ class XQueueInterface(object):
     def _login(self):
         payload = { 'username': self.auth['username'],
                     'password': self.auth['password'] }
-        return self._http_post(self.url+'/xqueue/login/', payload)
+        return self._http_post(self.url + '/xqueue/login/', payload)
 
 
     def _send_to_queue(self, header, body, files_to_upload):
         payload = {'xqueue_header': header,
                    'xqueue_body'  : body}
-        files = {} 
+        files = {}
         if files_to_upload is not None:
             for f in files_to_upload:
                 files.update({ f.name: f })
 
-        return self._http_post(self.url+'/xqueue/submit/', payload, files=files)
+        return self._http_post(self.url + '/xqueue/submit/', payload, files=files)
 
 
     def _http_post(self, url, data, files=None):
@@ -111,7 +113,7 @@ class XQueueInterface(object):
         except requests.exceptions.ConnectionError, err:
             log.error(err)
             return (1, 'cannot connect to server')
-        
+
         if r.status_code not in [200]:
             return (1, 'unexpected HTTP status code [%d]' % r.status_code)