third pass in capa cleanup: correct_map -> CorrectMap

  - added correctmap.py with CorrectMap class
  - messages subsumed into CorrectMap
  - response get_score called with old CorrectMap so hints based on history are possible

common/lib/capa/capa_problem.py

@@ -25,15 +25,14 @@ import struct
 from lxml import etree
 from xml.sax.saxutils import unescape
 
-from util import contextualize_text
-import inputtypes
-
-from responsetypes import NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse
-
 import calc
+from correctmap import CorrectMap
 import eia
+import inputtypes
+from util import contextualize_text
 
-log = logging.getLogger(__name__)
+# to be replaced with auto-registering
+from responsetypes import NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse
 
 # dict of tagname, Response Class -- this should come from auto-registering
 response_types = {'numericalresponse': NumericalResponse,
@@ -68,6 +67,12 @@ global_context = {'random': random,
 # These should be removed from HTML output, including all subelements
 html_problem_semantics = ["responseparam", "answer", "script"]
 
+#log = logging.getLogger(__name__)
+log = logging.getLogger('mitx.common.lib.capa.capa_problem')
+
+#-----------------------------------------------------------------------------
+# main class for this module
+
 class LoncapaProblem(object):
     '''
     Main class for capa Problems.
@@ -89,9 +94,7 @@ class LoncapaProblem(object):
         '''
 
         ## Initialize class variables from state
-        self.student_answers = dict()
-        self.correct_map = dict()
-        self.done = False
+        self.do_reset()
         self.problem_id = id
         self.system = system
         self.seed = seed
@@ -102,7 +105,7 @@ class LoncapaProblem(object):
             if 'student_answers' in state:
                 self.student_answers = state['student_answers']
             if 'correct_map' in state:
-                self.correct_map = state['correct_map']
+                self.correct_map.set_dict(state['correct_map'])
             if 'done' in state:
                 self.done = state['done']
 
@@ -125,7 +128,15 @@ class LoncapaProblem(object):
         # pre-parse the XML tree: modifies it to add ID's and perform some in-place transformations
         # this also creates the dict (self.responders) of Response instances for each question in the problem.
         # the dict has keys = xml subtree of Response, values = Response instance
-        self.preprocess_problem(self.tree, correct_map=self.correct_map, answer_map=self.student_answers)
+        self.preprocess_problem(self.tree, answer_map=self.student_answers)
+
+    def do_reset(self):
+        '''
+        Reset internal state to unfinished, with no answers
+        '''
+        self.student_answers = dict()
+        self.correct_map = CorrectMap()
+        self.done = False
 
     def __unicode__(self):
         return u"LoncapaProblem ({0})".format(self.fileobject)
@@ -134,9 +145,10 @@ class LoncapaProblem(object):
         ''' Stored per-user session data neeeded to:
             1) Recreate the problem
             2) Populate any student answers. '''
+
         return {'seed': self.seed,
                 'student_answers': self.student_answers,
-                'correct_map': self.correct_map,
+                'correct_map': self.correct_map.get_dict(),
                 'done': self.done}
 
     def get_max_score(self):
@@ -170,8 +182,12 @@ class LoncapaProblem(object):
         '''
         correct = 0
         for key in self.correct_map:
-            if self.correct_map[key] == u'correct':
-                correct += 1
+            try:
+                correct += self.correct_map.get_npoints(key)
+            except Exception,err:
+                log.error('key=%s, correct_map = %s' % (key,self.correct_map))
+                raise
+
         if (not self.student_answers) or len(self.student_answers) == 0:
             return {'score': 0,
                     'total': self.get_max_score()}
@@ -190,12 +206,14 @@ class LoncapaProblem(object):
         Calles the Response for each question in this problem, to do the actual grading.
         '''
         self.student_answers = answers
-        self.correct_map = dict()
-        log.info('%s: in grade_answers, answers=%s' % (self,answers))
+        oldcmap = self.correct_map				# old CorrectMap
+        newcmap = CorrectMap()					# start new with empty CorrectMap
         for responder in self.responders.values():
-            results = responder.get_score(answers)        # call the responsetype instance to do the actual grading
-            self.correct_map.update(results)
-        return self.correct_map
+            results = responder.get_score(answers,oldcmap)      # call the responsetype instance to do the actual grading
+            newcmap.update(results)
+        self.correct_map = newcmap
+        log.debug('%s: in grade_answers, answers=%s, cmap=%s' % (self,answers,newcmap))
+        return newcmap
 
     def get_question_answers(self):
         """Returns a dict of answer_ids to answer values. If we cannot generate
@@ -282,27 +300,17 @@ class LoncapaProblem(object):
             # but it will turn into a dict containing both the answer and any associated message
             # for the problem ID for the input element.
             status = "unsubmitted"
+            msg = ''
             if problemid in self.correct_map:
-                status = self.correct_map[problemtree.get('id')]
+                pid = problemtree.get('id')
+                status = self.correct_map.get_correctness(pid)
+                msg = self.correct_map.get_msg(pid)
 
             value = ""
             if self.student_answers and problemid in self.student_answers:
                 value = self.student_answers[problemid]
 
-            #### This code is a hack. It was merged to help bring two branches
-            #### in sync, but should be replaced. msg should be passed in a
-            #### response_type
-            # prepare the response message, if it exists in correct_map
-            if 'msg' in self.correct_map:
-                msg = self.correct_map['msg']
-            elif ('msg_%s' % problemid) in self.correct_map:
-                msg = self.correct_map['msg_%s' % problemid]
-            else:
-                msg = ''
-
             # do the rendering
-            # This should be broken out into a helper function
-            # that handles all input objects
             render_object = inputtypes.SimpleInput(system=self.system,
                                                    xml=problemtree,
                                                    state={'value': value,
@@ -333,7 +341,7 @@ class LoncapaProblem(object):
 
         return tree
 
-    def preprocess_problem(self, tree, correct_map=dict(), answer_map=dict()):  # private
+    def preprocess_problem(self, tree, answer_map=dict()):  # private
         '''
         Assign IDs to all the responses
         Assign sub-IDs to all entries (textline, schematic, etc.)
@@ -346,11 +354,8 @@ class LoncapaProblem(object):
 	self.responders = {}
         for response in tree.xpath('//' + "|//".join(response_types)):
             response_id_str = self.problem_id + "_" + str(response_id)
-            response.attrib['id'] = response_id_str				# create and save ID for this response
-
-            # if response_id not in correct_map: correct = 'unsubmitted'	# unused - to be removed
-            # response.attrib['state'] = correct
-            response_id += response_id
+            response.set('id',response_id_str)				# create and save ID for this response
+            response_id += 1
 
             answer_id = 1
             inputfields = tree.xpath("|".join(['//' + response.tag + '[@id=$id]//' + x for x in (entry_types + solution_types)]),

common/lib/capa/correctmap.py

+#-----------------------------------------------------------------------------
+# class used to store graded responses to CAPA questions
+#
+# Used by responsetypes and capa_problem
+
+class CorrectMap(object):
+    '''
+    Stores (correctness, npoints, msg) for each answer_id.
+    Behaves as a dict.
+    '''
+    cmap = {}
+
+    def __init__(self,*args,**kwargs):
+        self.set(*args,**kwargs)
+
+    def set(self,answer_id=None,correctness=None,npoints=None,msg=''):
+        if answer_id is not None:
+            self.cmap[answer_id] = {'correctness': correctness,
+                                    'npoints': npoints,
+                                    'msg': msg }
+
+    def __repr__(self):
+        return repr(self.cmap)
+
+    def get_dict(self):
+        '''
+        return dict version of self
+        '''
+        return self.cmap
+
+    def set_dict(self,correct_map):
+        '''
+        set internal dict to provided correct_map dict
+        for graceful migration, 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):
+            for k in self.cmap.keys(): self.cmap.pop(k)				# empty current dict
+            for k in correct_map: self.set(k,correct_map[k])			# create new dict entries
+        else:
+            self.cmap = correct_map
+
+    def is_correct(self,answer_id):
+        if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct'
+        return None
+
+    def get_npoints(self,answer_id):
+        if self.is_correct(answer_id):
+            npoints = self.cmap[answer_id].get('npoints',1)	# default to 1 point if correct
+            return npoints or 1
+        return 0						# if not correct, 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}
+
+    def get_property(self,answer_id,property,default=None):
+        if answer_id in self.cmap: return self.cmap[answer_id].get(property,default)
+        return default
+
+    def get_correctness(self,answer_id):
+        return self.get_property(answer_id,'correctness')
+
+    def get_msg(self,answer_id):
+        return self.get_property(answer_id,'msg','')
+
+    def update(self,other_cmap):
+        '''
+        Update this CorrectMap with the contents of another CorrectMap
+        '''
+        if not isinstance(other_cmap,CorrectMap):
+            raise Exception('CorrectMap.update called with invalid argument %s' % other_cmap)
+        self.cmap.update(other_cmap.get_dict())
+
+    __getitem__ = cmap.__getitem__
+    __iter__ = cmap.__iter__
+    items = cmap.items
+    keys = cmap.keys
+
+    

common/lib/capa/responsetypes.py

@@ -21,6 +21,7 @@ import abc
 
 # specific library imports
 from calc import evaluator, UndefinedVariable
+from correctmap import CorrectMap
 from util import *
 from lxml import etree
 from lxml.html.soupparser import fromstring as fromstring_bs	# uses Beautiful Soup!!! FIXME?
@@ -53,7 +54,7 @@ class StudentInputError(Exception):
 class GenericResponse(object):
     '''
     Base class for CAPA responsetypes.  Each response type (ie a capa question,
-    which is part of a capa problem) is represented as a superclass,
+    which is part of a capa problem) is represented as a subclass,
     which should provide the following methods:
 
       - get_score           : evaluate the given student answers, and return a CorrectMap
@@ -140,10 +141,16 @@ class GenericResponse(object):
         return tree
 
     @abc.abstractmethod
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         '''
         Return a CorrectMap for the answers expected vs given.  This includes
         (correctness, npoints, msg) for each answer_id.
+
+        Arguments:
+
+         - student_answers : dict of (answer_id,answer) where answer = student input (string)
+         - old_cmap        : previous CorrectMap (may be empty); useful for analyzing or recording history of responses
+
         '''
         pass
 
@@ -201,15 +208,15 @@ class MultipleChoiceResponse(GenericResponse):
                 else:
                     choice.set("name", "choice_"+choice.get("name"))
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         '''
         grade student response.
         '''
         # log.debug('%s: student_answers=%s, correct_choices=%s' % (unicode(self),student_answers,self.correct_choices))
         if self.answer_id in student_answers and student_answers[self.answer_id] in self.correct_choices:
-            return {self.answer_id:'correct'}
+            return CorrectMap(self.answer_id,'correct')
         else:
-            return {self.answer_id:'incorrect'}
+            return CorrectMap(self.answer_id,'incorrect')
 
     def get_answers(self):
         return {self.answer_id:self.correct_choices}
@@ -226,14 +233,14 @@ class TrueFalseResponse(MultipleChoiceResponse):
                 else:
                     choice.set("name", "choice_"+choice.get("name"))
     
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         correct = set(self.correct_choices)
         answers = set(student_answers.get(self.answer_id, []))
         
         if correct == answers:
-            return { self.answer_id : 'correct'}
+            return CorrectMap( self.answer_id , 'correct')
         
-        return {self.answer_id : 'incorrect'}
+        return CorrectMap(self.answer_id ,'incorrect')
 
 #-----------------------------------------------------------------------------
 
@@ -251,15 +258,15 @@ class OptionResponse(GenericResponse):
     def setup_response(self):
         self.answer_fields = self.inputfields
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         # log.debug('%s: student_answers=%s' % (unicode(self),student_answers))
-        cmap = {}
+        cmap = CorrectMap()
         amap = self.get_answers()
         for aid in amap:
             if aid in student_answers and student_answers[aid]==amap[aid]:
-                cmap[aid] = 'correct'
+                cmap.set(aid,'correct')
             else:
-                cmap[aid] = 'incorrect'
+                cmap.set(aid,'incorrect')
         return cmap
 
     def get_answers(self):
@@ -291,7 +298,7 @@ class NumericalResponse(GenericResponse):
         except Exception:
             self.answer_id = None
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         '''Grade a numeric response '''
         student_answer = student_answers[self.answer_id]
         try:
@@ -303,9 +310,9 @@ class NumericalResponse(GenericResponse):
             raise StudentInputError('Invalid input -- please use a number only')
 
         if correct:
-            return {self.answer_id:'correct'}
+            return CorrectMap(self.answer_id,'correct')
         else:
-            return {self.answer_id:'incorrect'}
+            return CorrectMap(self.answer_id,'incorrect')
 
     def get_answers(self):
         return {self.answer_id:self.correct_answer}
@@ -395,7 +402,7 @@ def sympy_check2():
                 else:
                     self.code = answer.text
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         '''
         student_answers is a dict with everything from request.POST, but with the first part
         of each key removed (the string before the first "_").
@@ -495,12 +502,10 @@ def sympy_check2():
                 correct = ['correct']*len(idset) if ret else ['incorrect']*len(idset)
 
         # build map giving "correct"ness of the answer(s)
-        #correct_map = dict(zip(idset, self.context['correct']))
-        correct_map = {}
+        correct_map = CorrectMap()
         for k in range(len(idset)):
-            correct_map[idset[k]] = correct[k]
-            correct_map['msg_%s' % idset[k]] = messages[k]
-        return  correct_map
+            correct_map.set(idset[k], correct[k], msg=messages[k])
+        return correct_map
 
     def get_answers(self):
         '''
@@ -642,9 +647,11 @@ main()
 
         return rxml
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
+        idset = sorted(self.answer_ids)
+        cmap = CorrectMap()
         try:
-            submission = [student_answers[k] for k in sorted(self.answer_ids)]
+            submission = [student_answers[k] for k in idset]
         except Exception,err:
             log.error('Error %s: cannot get student answer for %s; student_answers=%s' % (err,self.answer_ids,student_answers))
             raise Exception,err
@@ -658,9 +665,9 @@ main()
         except Exception, err:
             log.error('Error %s' % err)
             if self.system.DEBUG:
-                correct_map = dict(zip(sorted(self.answer_ids), ['incorrect'] * len(self.answer_ids) ))
-                correct_map['msg_%s' % self.answer_ids[0]] = '<font color="red" size="+2">%s</font>' % str(err).replace('<','&lt;')
-                return correct_map
+                cmap.set_dict(dict(zip(sorted(self.answer_ids), ['incorrect'] * len(idset) )))
+                cmap.set_property(self.answer_ids[0],'msg','<font color="red" size="+2">%s</font>' % str(err).replace('<','&lt;'))
+                return cmap
 
         ad = rxml.find('awarddetail').text
         admap = {'EXACT_ANS':'correct',         # TODO: handle other loncapa responses
@@ -670,13 +677,13 @@ main()
         if ad in admap:
             self.context['correct'][0] = admap[ad]
 
-        # self.context['correct'] = ['correct','correct']
-        correct_map = dict(zip(sorted(self.answer_ids), self.context['correct']))
-        
-        # store message in correct_map
-        correct_map['msg_%s' % self.answer_ids[0]] = rxml.find('message').text.replace('&nbsp;','&#160;')  
+        # create CorrectMap
+        for key in idset:
+            idx = idset.index(key)
+            msg = rxml.find('message').text.replace('&nbsp;','&#160;') if idx==0 else None
+            cmap.set(key, self.context['correct'][idx], msg=msg)
 
-        return  correct_map
+        return cmap
 
     def get_answers(self):
         '''
@@ -750,7 +757,7 @@ class FormulaResponse(GenericResponse):
         else: # Default
             self.case_sensitive = False
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         variables=self.samples.split('@')[0].split(',')
         numsamples=int(self.samples.split('@')[1].split('#')[1])
         sranges=zip(*map(lambda x:map(float, x.split(",")), 
@@ -776,11 +783,11 @@ class FormulaResponse(GenericResponse):
                 #traceback.print_exc()
                 raise StudentInputError("Error in formula")
             if numpy.isnan(student_result) or numpy.isinf(student_result):
-                return {self.answer_id:"incorrect"}
+                return CorrectMap(self.answer_id, "incorrect")
             if not compare_with_tolerance(student_result, instructor_result, self.tolerance):
-                return {self.answer_id:"incorrect"}
+                return CorrectMap(self.answer_id, "incorrect")
  
-        return {self.answer_id:"correct"}
+        return CorrectMap(self.answer_id, "correct")
 
     def strip_dict(self, d):
         ''' Takes a dict. Returns an identical dict, with all non-word
@@ -810,12 +817,13 @@ class SchematicResponse(GenericResponse):
         else:
             self.code = answer.text
 
-    def get_score(self, student_answers):
+    def get_score(self, student_answers, old_cmap):
         from capa_problem import global_context
         submission = [json.loads(student_answers[k]) for k in sorted(self.answer_ids)]
         self.context.update({'submission':submission})
         exec self.code in global_context, self.context
-        return  zip(sorted(self.answer_ids), self.context['correct'])
+        cmap = CorrectMap()
+        return  cmap.set_dict(zip(sorted(self.answer_ids), self.context['correct']))
 
     def get_answers(self):
         # use answers provided in input elements
@@ -845,8 +853,8 @@ class ImageResponse(GenericResponse):
         self.ielements = self.inputfields
         self.answer_ids = [ie.get('id')  for ie in self.ielements]
 
-    def get_score(self, student_answers):
-        correct_map = {}
+    def get_score(self, student_answers, old_cmap):
+        correct_map = CorrectMap()
         expectedset = self.get_answers()
 
         for aid in self.answer_ids:	# loop through IDs of <imageinput> fields in our stanza
@@ -869,9 +877,9 @@ class ImageResponse(GenericResponse):
             
             # answer is correct if (x,y) is within the specified rectangle
             if (llx <= gx <= urx) and (lly <= gy <= ury):
-                correct_map[aid] = 'correct'
+                correct_map.set(aid, 'correct')
             else:
-                correct_map[aid] = 'incorrect'
+                correct_map.set(aid, 'incorrect')
         return correct_map
 
     def get_answers(self):

common/lib/xmodule/capa_module.py

@@ -13,6 +13,7 @@ from lxml import etree
 from x_module import XModule, XModuleDescriptor
 from capa.capa_problem import LoncapaProblem
 from capa.responsetypes import StudentInputError
+
 log = logging.getLogger("mitx.courseware")
 
 #-----------------------------------------------------------------------------
@@ -365,18 +366,17 @@ class Module(XModule):
         self.attempts = self.attempts + 1
         self.lcp.done=True
 
-        success = 'correct'
-        for i in correct_map:
-            if correct_map[i]!='correct':
+        success = 'correct'				# success = correct if ALL questions in this problem are correct
+        for answer_id in correct_map:
+            if not correct_map.is_correct(answer_id):
                 success = 'incorrect'
 
-        event_info['correct_map']=correct_map
+        event_info['correct_map']=correct_map.get_dict()	# log this in the tracker
         event_info['success']=success
-
         self.tracker('save_problem_check', event_info)
 
         try:
-            html = self.get_problem_html(encapsulate=False)
+            html = self.get_problem_html(encapsulate=False)	# render problem into HTML
         except Exception,err:
             log.error('failed to generate html')
             raise Exception,err
@@ -430,17 +430,10 @@ class Module(XModule):
             self.tracker('reset_problem_fail', event_info)
             return "Refresh the page and make an attempt before resetting."
 
-        self.lcp.done=False
-        self.lcp.answers=dict()
-        self.lcp.correct_map=dict()
-        self.lcp.student_answers = dict()
-
-
+        self.lcp.do_reset()		# call method in LoncapaProblem to reset itself
         if self.rerandomize == "always":
-            self.lcp.context=dict()
-            self.lcp.questions=dict() # Detailed info about questions in problem instance. TODO: Should be by id and not lid.
-            self.lcp.seed=None
-
+            self.lcp.seed=None		# reset random number generator seed (note the self.lcp.get_state() in next line)
+ 
         self.lcp=LoncapaProblem(self.filestore.open(self.filename), self.item_id, self.lcp.get_state(), system=self.system)
 
         event_info['new_state']=self.lcp.get_state()