diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index a1b059b889..506e3a829a 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -29,6 +29,7 @@ setup( "error = xmodule.error_module:ErrorDescriptor", "problem = xmodule.capa_module:CapaDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", + "randomize = xmodule.randomize_module:RandomizeDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 8d04daa563..f4b0c32e96 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -2,6 +2,7 @@ import cgi import datetime import dateutil import dateutil.parser +import hashlib import json import logging import traceback @@ -25,6 +26,22 @@ log = logging.getLogger("mitx.courseware") #----------------------------------------------------------------------------- TIMEDELTA_REGEX = re.compile(r'^((?P\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\d+?) second(?:s)?)?$') +# Generated this many different variants of problems with rerandomize=per_student +NUM_RANDOMIZATION_BINS = 20 + +def randomization_bin(seed, problem_id): + """ + Pick a randomization bin for the problem given the user's seed and a problem id. + + We do this because we only want e.g. 20 randomizations of a problem to make analytics + interesting. To avoid having sets of students that always get the same problems, + we'll combine the system's per-student seed with the problem id in picking the bin. + """ + h = hashlib.sha1() + h.update(str(seed)) + h.update(str(problem_id)) + # get the first few digits of the hash, convert to an int, then mod. + return int(h.hexdigest()[:7], 16) % NUM_RANDOMIZATION_BINS def only_one(lst, default="", process=lambda x: x): """ @@ -138,13 +155,9 @@ class CapaModule(XModule): if self.rerandomize == 'never': self.seed = 1 - elif self.rerandomize == "per_student" and hasattr(self.system, 'id'): - # TODO: This line is badly broken: - # (1) We're passing student ID to xmodule. - # (2) There aren't bins of students. -- we only want 10 or 20 randomizations, and want to assign students - # to these bins, and may not want cohorts. So e.g. hash(your-id, problem_id) % num_bins. - # - analytics really needs small number of bins. - self.seed = system.id + elif self.rerandomize == "per_student" and hasattr(self.system, 'seed'): + # see comment on randomization_bin + self.seed = randomization_bin(system.seed, self.location.url) else: self.seed = None diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py new file mode 100644 index 0000000000..f88cdc5efb --- /dev/null +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -0,0 +1,121 @@ +import json +import logging +import random + +from xmodule.mako_module import MakoModuleDescriptor +from xmodule.x_module import XModule +from xmodule.xml_module import XmlDescriptor +from xmodule.modulestore import Location +from xmodule.seq_module import SequenceDescriptor + +from pkg_resources import resource_string + +log = logging.getLogger('mitx.' + __name__) + +class RandomizeModule(XModule): + """ + Chooses a random child module. Chooses the same one every time for each student. + + Example: + + + + + + + User notes: + + - If you're randomizing amongst graded modules, each of them MUST be worth the same + number of points. Otherwise, the earth will be overrun by monsters from the + deeps. You have been warned. + + Technical notes: + - There is more dark magic in this code than I'd like. The whole varying-children + + grading interaction is a tangle between super and subclasses of descriptors and + modules. +""" + + def __init__(self, system, location, definition, descriptor, + instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, + instance_state, shared_state, **kwargs) + + # NOTE: calling self.get_children() creates a circular reference-- + # it calls get_child_descriptors() internally, but that doesn't work until + # we've picked a choice + num_choices = len(self.descriptor.get_children()) + + self.choice = None + if instance_state is not None: + state = json.loads(instance_state) + self.choice = state.get('choice', None) + if self.choice > num_choices: + # Oops. Children changed. Reset. + self.choice = None + + if self.choice is None: + # choose one based on the system seed, or randomly if that's not available + if num_choices > 0: + if system.seed is not None: + self.choice = system.seed % num_choices + else: + self.choice = random.randrange(0, num_choices) + + if self.choice is not None: + self.child_descriptor = self.descriptor.get_children()[self.choice] + # Now get_children() should return a list with one element + log.debug("children of randomize module (should be only 1): %s", + self.get_children()) + self.child = self.get_children()[0] + else: + self.child_descriptor = None + self.child = None + + + def get_instance_state(self): + return json.dumps({'choice': self.choice}) + + + def get_child_descriptors(self): + """ + For grading--return just the chosen child. + """ + if self.child_descriptor is None: + return [] + + return [self.child_descriptor] + + + def get_html(self): + if self.child is None: + # raise error instead? In fact, could complain on descriptor load... + return "
Nothing to randomize between
" + + return self.child.get_html() + + def get_icon_class(self): + return self.child.get_icon_class() if self.child else 'other' + + +class RandomizeDescriptor(SequenceDescriptor): + # the editing interface can be the same as for sequences -- just a container + module_class = RandomizeModule + + filename_extension = "xml" + + stores_state = True + + def definition_to_xml(self, resource_fs): + xml_object = etree.Element('randomize') + for child in self.get_children(): + xml_object.append( + etree.fromstring(child.export_to_xml(resource_fs))) + return xml_object + + def has_dynamic_children(self): + """ + Grading needs to know that only one of the children is actually "real". This + makes it use module.get_child_descriptors(). + """ + return True + diff --git a/common/lib/xmodule/xmodule/tests/test_randomize_module.py b/common/lib/xmodule/xmodule/tests/test_randomize_module.py new file mode 100644 index 0000000000..6353245f1a --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_randomize_module.py @@ -0,0 +1,55 @@ +import unittest +from time import strptime +from fs.memoryfs import MemoryFS + +from mock import Mock, patch + +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore + + +ORG = 'test_org' +COURSE = 'test_course' + +START = '2013-01-01T01:00:00' + + +from test_course_module import DummySystem as DummyImportSystem +from . import test_system + + +class RandomizeModuleTestCase(unittest.TestCase): + """Make sure the randomize module works""" + @staticmethod + def get_dummy_course(start): + """Get a dummy course""" + + system = DummyImportSystem(load_error_modules=True) + + def to_attrb(n, v): + return '' if v is None else '{0}="{1}"'.format(n, v).lower() + + start_xml = ''' + + + + Two houses, ... + Three houses, ... + + + + '''.format(org=ORG, course=COURSE, start=start) + + return system.process_xml(start_xml) + + def test_import(self): + """ + Just make sure descriptor loads without error + """ + descriptor = self.get_dummy_course(START) + + # TODO: add tests that create a module and check. Passing state is a good way to + # check that child access works... + diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index 397bd3e136..14105b41d0 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -48,3 +48,5 @@ class VerticalDescriptor(SequenceDescriptor): js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} js_module_name = "VerticalDescriptor" + # TODO (victor): Does this need its own definition_to_xml method? Otherwise it looks + # like verticals will get exported as sequentials...