From 408e988fbc59fd1e648cf46db4b4e7022d3cd37c Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 3 Aug 2012 11:41:06 -0400 Subject: [PATCH 01/51] Started working on fast_grade method. It skips loading sections that a student doesn't have any answers for. --- lms/djangoapps/courseware/grades.py | 106 ++++++++++++++++++++++++++-- lms/djangoapps/courseware/views.py | 13 ++++ lms/urls.py | 4 ++ 3 files changed, 116 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 5a817e3d6c..ca8620c1f6 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -3,6 +3,7 @@ import logging from django.conf import settings +from module_render import get_module from xmodule import graders from xmodule.graders import Score from models import StudentModule @@ -10,6 +11,103 @@ from models import StudentModule _log = logging.getLogger("mitx.courseware") + + + +def get_graded_sections(course_descriptor): + """ + Arguments: + course_descriptor: a CourseDescriptor object for the course to be graded + + Returns: + A dictionary keyed by section-type. The values are arrays of dictionaries containing + "section_descriptor" : The section descriptor + "xmoduledescriptors" : An array of xmoduledescriptors that could possibly be in the section, for any student + """ + + graded_sections = {} + for c in course_descriptor.get_children(): + sections = [] + for s in c.get_children(): + if s.metadata.get('graded', False): + + xmoduledescriptors = [] + for module in yield_descriptor_descendents(s): + xmoduledescriptors.append(module) + + section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : xmoduledescriptors} + + section_format = s.metadata.get('format', "") + graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description] + + return graded_sections + +def yield_descriptor_descendents(module_descriptor): + for child in module_descriptor.get_children(): + yield child + for module_descriptor in yield_descriptor_descendents(child): + yield module_descriptor + +def yield_module_descendents(module): + for child in module.get_display_items(): + yield child + for module in yield_module_descendents(child): + yield module + +def fast_grade(student, request, course_graded_sections, grader, student_module_cache): + totaled_scores = {} + for section_format, sections in course_graded_sections.iteritems(): + format_scores = [] + for section in sections: + section_descriptor = section['section_descriptor'] + section_name = section_descriptor.metadata.get('display_name') + + should_grade_section = False + # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0% + for moduledescriptor in section['xmoduledescriptors']: + if student_module_cache.lookup(moduledescriptor.category, moduledescriptor.location.url() ): + should_grade_section = True + break + + if should_grade_section: + scores = [] + (section_module, _, _, _) = get_module(student, request, section_descriptor.location, student_module_cache) + + for module in yield_descriptor_descendents(section_module): + (correct, total) = get_score(student, module, student_module_cache) + graded = module.metadata.get("graded", False) + + if correct is None and total is None: + continue + + if settings.GENERATE_PROFILE_SCORES: + if total > 1: + correct = random.randrange(max(total - 2, 1), total + 1) + else: + correct = total + + if not total > 0: + #We simply cannot grade a problem that is 12/0, because we might need it as a percentage + graded = False + + scores.append(Score(correct, total, graded, module.metadata.get('display_name'))) + + section_total, graded_total = graders.aggregate_scores(scores, section_name) + else: + section_total = Score(0.0, 1.0, False, section_name) + graded_possible = 1.0 #if s.metadata.get("graded", False) else 0.0 + graded_total = Score(0.0, graded_possible, True, section_name) + + #Add the graded total to totaled_scores + if graded_total.possible > 0: + format_scores.append(graded_total) + + totaled_scores[section_format] = format_scores + + grade_summary = grade_summary = grader.grade(totaled_scores) + return grade_summary + + def grade_sheet(student, course, grader, student_module_cache): """ This pulls a summary of all problems in the course. It returns a dictionary with two datastructures: @@ -30,15 +128,9 @@ def grade_sheet(student, course, grader, student_module_cache): for c in course.get_children(): sections = [] for s in c.get_children(): - def yield_descendents(module): - yield module - for child in module.get_display_items(): - for module in yield_descendents(child): - yield module - graded = s.metadata.get('graded', False) scores = [] - for module in yield_descendents(s): + for module in yield_module_descendents(s): (correct, total) = get_score(student, module, student_module_cache) if correct is None and total is None: diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 7281ab01ad..a89a4bf34f 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -35,6 +35,19 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} +def test_grading(request, course_id): + course = check_course(course_id) + + sections = grades.get_graded_sections(course) + + student_module_cache = StudentModuleCache(request.user, course) + + grade_result = grades.fast_grade(request.user, request, sections, course.grader, student_module_cache) + + return HttpResponse("" + json.dumps(grade_result) + "") + + + def user_groups(user): if not user.is_authenticated(): return [] diff --git a/lms/urls.py b/lms/urls.py index 1c4a065e2b..8d7460ddc2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -118,6 +118,10 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/about$', 'courseware.views.course_about', name="about_course"), + # TODO: Important. Remove this after testing! + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade$', + 'courseware.views.test_grading', name="info"), + #Inside the course url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), From 854508b0067f9617096bbddf6f3d81ebefe1d22f Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 3 Aug 2012 14:30:45 -0400 Subject: [PATCH 02/51] Removed the unnecessary creation of StudentModules when calling module_render's get_module. --- lms/djangoapps/courseware/grades.py | 33 +++--- .../management/commands/check_course.py | 2 +- lms/djangoapps/courseware/module_render.py | 108 ++++++++++++------ lms/djangoapps/courseware/views.py | 6 +- 4 files changed, 90 insertions(+), 59 deletions(-) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index ca8620c1f6..e522468f45 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -3,17 +3,13 @@ import logging from django.conf import settings -from module_render import get_module +from module_render import get_module, get_instance_module from xmodule import graders from xmodule.graders import Score from models import StudentModule _log = logging.getLogger("mitx.courseware") - - - - def get_graded_sections(course_descriptor): """ Arguments: @@ -71,7 +67,7 @@ def fast_grade(student, request, course_graded_sections, grader, student_module_ if should_grade_section: scores = [] - (section_module, _, _, _) = get_module(student, request, section_descriptor.location, student_module_cache) + section_module = get_module(student, request, section_descriptor.location, student_module_cache) for module in yield_descriptor_descendents(section_module): (correct, total) = get_score(student, module, student_module_cache) @@ -175,7 +171,7 @@ def grade_sheet(student, course, grader, student_module_cache): 'grade_summary': grade_summary} -def get_score(user, problem, cache): +def get_score(user, problem, student_module_cache): """ Return the score for a user on a problem @@ -186,17 +182,18 @@ def get_score(user, problem, cache): correct = 0.0 # If the ID is not in the cache, add the item - instance_module = cache.lookup(problem.category, problem.id) - if instance_module is None: - instance_module = StudentModule(module_type=problem.category, - module_state_key=problem.id, - student=user, - state=None, - grade=0, - max_grade=problem.max_score(), - done='i') - cache.append(instance_module) - instance_module.save() + instance_module = get_instance_module(user, problem, student_module_cache) + # instance_module = student_module_cache.lookup(problem.category, problem.id) + # if instance_module is None: + # instance_module = StudentModule(module_type=problem.category, + # module_state_key=problem.id, + # student=user, + # state=None, + # grade=0, + # max_grade=problem.max_score(), + # done='i') + # cache.append(instance_module) + # instance_module.save() # If this problem is ungraded/ungradable, bail if instance_module.max_grade is None: diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py index 6ccd6d5fe7..bf54147513 100644 --- a/lms/djangoapps/courseware/management/commands/check_course.py +++ b/lms/djangoapps/courseware/management/commands/check_course.py @@ -79,7 +79,7 @@ class Command(BaseCommand): # TODO (cpennington): Get coursename in a legitimate way course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012' student_module_cache = StudentModuleCache(sample_user, modulestore().get_item(course_location)) - (course, _, _, _) = get_module(sample_user, None, course_location, student_module_cache) + course = get_module(sample_user, None, course_location, student_module_cache) to_run = [ #TODO (vshnayder) : make check_rendering work (use module_render.py), diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index bf7d1f4c51..021f2b2ed8 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -45,9 +45,9 @@ def toc_for_course(user, request, course, active_chapter, active_section): chapters with name 'hidden' are skipped. ''' - + student_module_cache = StudentModuleCache(user, course, depth=2) - (course, _, _, _) = get_module(user, request, course.location, student_module_cache) + course = get_module(user, request, course.location, student_module_cache) chapters = list() for chapter in course.get_display_items(): @@ -114,25 +114,26 @@ def get_module(user, request, location, student_module_cache, position=None): - position : extra information from URL for user-specified position within module - Returns: - - a tuple (xmodule instance, instance_module, shared_module, module category). - instance_module is a StudentModule specific to this module for this student, - or None if this is an anonymous user - shared_module is a StudentModule specific to all modules with the same - 'shared_state_key' attribute, or None if the module does not elect to - share state + Returns: xmodule instance + ''' descriptor = modulestore().get_item(location) - - instance_module = student_module_cache.lookup(descriptor.category, - descriptor.location.url()) - - shared_state_key = getattr(descriptor, 'shared_state_key', None) - if shared_state_key is not None: - shared_module = student_module_cache.lookup(descriptor.category, - shared_state_key) + + #TODO Only check the cache if this module can possibly have state + if user.is_authenticated(): + instance_module = student_module_cache.lookup(descriptor.category, + descriptor.location.url()) + + shared_state_key = getattr(descriptor, 'shared_state_key', None) + if shared_state_key is not None: + shared_module = student_module_cache.lookup(descriptor.category, + shared_state_key) + else: + shared_module = None else: + instance_module = None shared_module = None + instance_state = instance_module.state if instance_module is not None else None shared_state = shared_module.state if shared_module is not None else None @@ -144,9 +145,8 @@ def get_module(user, request, location, student_module_cache, position=None): str(user.id) + '/' + descriptor.location.url() + '/') def _get_module(location): - (module, _, _, _) = get_module(user, request, location, + return get_module(user, request, location, student_module_cache, position) - return module # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory @@ -177,30 +177,59 @@ def get_module(user, request, location, student_module_cache, position=None): if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: module.get_html = add_histogram(module.get_html) - # If StudentModule for this instance wasn't already in the database, - # and this isn't a guest user, create it. + return module + +def get_instance_module(user, module, student_module_cache): + """ + Returns instance_module is a StudentModule specific to this module for this student, + or None if this is an anonymous user + """ if user.is_authenticated(): + instance_module = student_module_cache.lookup(module.category, + module.location.url()) + if not instance_module: instance_module = StudentModule( student=user, - module_type=descriptor.category, + module_type=module.category, module_state_key=module.id, state=module.get_instance_state(), max_grade=module.max_score()) instance_module.save() - # Add to cache. The caller and the system context have references - # to it, so the change persists past the return student_module_cache.append(instance_module) - if not shared_module and shared_state_key is not None: - shared_module = StudentModule( - student=user, - module_type=descriptor.category, - module_state_key=shared_state_key, - state=module.get_shared_state()) - shared_module.save() - student_module_cache.append(shared_module) - - return (module, instance_module, shared_module, descriptor.category) + + return instance_module + else: + return None + +def get_shared_instance_module(user, module, student_module_cache): + """ + Return shared_module is a StudentModule specific to all modules with the same + 'shared_state_key' attribute, or None if the module does not elect to + share state + """ + if user.is_authenticated(): + # To get the shared_state_key, we need to descriptor + descriptor = modulestore().get_item(module.location) + + shared_state_key = getattr(module, 'shared_state_key', None) + if shared_state_key is not None: + shared_module = student_module_cache.lookup(module.category, + shared_state_key) + if not shared_module: + shared_module = StudentModule( + student=user, + module_type=descriptor.category, + module_state_key=shared_state_key, + state=module.get_shared_state()) + shared_module.save() + student_module_cache.append(shared_module) + else: + shared_module = None + + return shared_module + else: + return None # TODO: TEMPORARY BYPASS OF AUTH! @@ -218,7 +247,9 @@ def xqueue_callback(request, userid, id, dispatch): user = User.objects.get(id=userid) student_module_cache = StudentModuleCache(user, modulestore().get_item(id)) - instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache) + instance = get_module(request.user, request, id, student_module_cache) + + instance_module = get_instance_module(request.user, instance, student_module_cache) if instance_module is None: log.debug("Couldn't find module '%s' for user '%s'", @@ -264,8 +295,11 @@ def modx_dispatch(request, dispatch=None, id=None): # ''' (fix emacs broken parsing) student_module_cache = StudentModuleCache(request.user, modulestore().get_item(id)) - instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache) - + instance = get_module(request.user, request, id, student_module_cache) + + instance_module = get_instance_module(request.user, instance, student_module_cache) + shared_module = get_shared_instance_module(request.user, instance, student_module_cache) + # Don't track state for anonymous users (who don't have student modules) if instance_module is not None: oldgrade = instance_module.grade diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index a89a4bf34f..423d436087 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -93,7 +93,7 @@ def gradebook(request, course_id): for student in student_objects: student_module_cache = StudentModuleCache(student, course) - course, _, _, _ = get_module(request.user, request, course.location, student_module_cache) + course = get_module(request.user, request, course.location, student_module_cache) student_info.append({ 'username': student.username, 'id': student.id, @@ -122,7 +122,7 @@ def profile(request, course_id, student_id=None): user_info = UserProfile.objects.get(user=student) student_module_cache = StudentModuleCache(request.user, course) - course_module, _, _, _ = get_module(request.user, request, course.location, student_module_cache) + course_module = get_module(request.user, request, course.location, student_module_cache) context = {'name': user_info.name, 'username': student.username, @@ -213,7 +213,7 @@ def index(request, course_id, chapter=None, section=None, if section_descriptor is not None: student_module_cache = StudentModuleCache(request.user, section_descriptor) - module, _, _, _ = get_module(request.user, request, + module = get_module(request.user, request, section_descriptor.location, student_module_cache) context['content'] = module.get_html() From 694520ebb2c258b225b1d1e18fddf265039a1541 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 3 Aug 2012 17:00:04 -0400 Subject: [PATCH 03/51] StudentModuleCache now allows a list of descriptors. This speeds up grading significantly. --- common/lib/xmodule/xmodule/x_module.py | 10 ++-- lms/djangoapps/courseware/grades.py | 6 ++- lms/djangoapps/courseware/models.py | 67 ++++++++++++++++++-------- lms/djangoapps/courseware/views.py | 5 +- 4 files changed, 58 insertions(+), 30 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 3406bcb99c..30522b871c 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -501,11 +501,11 @@ class XModuleDescriptor(Plugin, HTMLSnippet): all(getattr(self, attr, None) == getattr(other, attr, None) for attr in self.equality_attributes)) - if not eq: - for attr in self.equality_attributes: - print(getattr(self, attr, None), - getattr(other, attr, None), - getattr(self, attr, None) == getattr(other, attr, None)) + # if not eq: + # for attr in self.equality_attributes: + # print(getattr(self, attr, None), + # getattr(other, attr, None), + # getattr(self, attr, None) == getattr(other, attr, None)) return eq diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index e522468f45..7cb928e648 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -20,6 +20,7 @@ def get_graded_sections(course_descriptor): "section_descriptor" : The section descriptor "xmoduledescriptors" : An array of xmoduledescriptors that could possibly be in the section, for any student """ + all_descriptors = [] graded_sections = {} for c in course_descriptor.get_children(): @@ -35,8 +36,11 @@ def get_graded_sections(course_descriptor): section_format = s.metadata.get('format', "") graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description] + + all_descriptors.extend(xmoduledescriptors) + all_descriptors.append(s) - return graded_sections + return graded_sections, all_descriptors def yield_descriptor_descendents(module_descriptor): for child in module_descriptor.get_children(): diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 3b0ca7fdcf..79617e428d 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -67,17 +67,30 @@ class StudentModuleCache(object): """ A cache of StudentModules for a specific student """ - def __init__(self, user, descriptor, depth=None): + def __init__(self, user, descriptor=None, depth=None, descriptors=None, descriptor_filter=lambda descriptor: True): ''' Find any StudentModule objects that are needed by any child modules of the - supplied descriptor. Avoids making multiple queries to the database - - descriptor: An XModuleDescriptor - depth is the number of levels of descendent modules to load StudentModules for, in addition to - the supplied descriptor. If depth is None, load all descendent StudentModules + supplied descriptor, or caches only the StudentModule objects specifically + for every descriptor in descriptors. Avoids making multiple queries to the + database. + + There are two ways to init: + descriptor: An XModuleDescriptor + depth is the number of levels of descendent modules to load StudentModules for, in addition to + the supplied descriptor. If depth is None, load all descendent StudentModules + OR + descriptors: An array of XModuleDescriptors. + + descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + should be cached ''' if user.is_authenticated(): - module_ids = self._get_module_state_keys(descriptor, depth) + if not (descriptor == None) != (descriptors == None): #An xor on the descriptor and descriptors parameters. + raise ValueError("Either the descriptor or descriptors must be supplied to StudentModuleCache.") + + if descriptor != None: + descriptors = self._get_child_descriptors(descriptor, depth) + module_ids = self._get_module_state_keys(descriptors, descriptor_filter) # This works around a limitation in sqlite3 on the number of parameters # that can be put into a single query @@ -91,27 +104,39 @@ class StudentModuleCache(object): else: self.cache = [] - - def _get_module_state_keys(self, descriptor, depth): - ''' - Get a list of the state_keys needed for StudentModules - required for this module descriptor - + + def _get_child_descriptors(self, descriptor, depth): + """ descriptor: An XModuleDescriptor depth is the number of levels of descendent modules to load StudentModules for, in addition to the supplied descriptor. If depth is None, load all descendent StudentModules - ''' - keys = [descriptor.location.url()] - - shared_state_key = getattr(descriptor, 'shared_state_key', None) - if shared_state_key is not None: - keys.append(shared_state_key) - + """ + descriptors = [descriptor] + if depth is None or depth > 0: new_depth = depth - 1 if depth is not None else depth for child in descriptor.get_children(): - keys.extend(self._get_module_state_keys(child, new_depth)) + descriptors.extend(self._get_child_descriptors(child, new_depth)) + + return descriptors + + def _get_module_state_keys(self, descriptors, descriptor_filter): + ''' + Get a list of the state_keys needed for StudentModules + required for this module descriptor + + descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + should be cached + ''' + keys = [] + for descriptor in descriptors: + if descriptor_filter(descriptor): + keys.append(descriptor.location.url()) + + shared_state_key = getattr(descriptor, 'shared_state_key', None) + if shared_state_key is not None: + keys.append(shared_state_key) return keys diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 423d436087..c8939c0b75 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -38,9 +38,8 @@ template_imports = {'urllib': urllib} def test_grading(request, course_id): course = check_course(course_id) - sections = grades.get_graded_sections(course) - - student_module_cache = StudentModuleCache(request.user, course) + sections, all_descriptors = grades.get_graded_sections(course) + student_module_cache = StudentModuleCache(request.user, descriptors=all_descriptors) grade_result = grades.fast_grade(request.user, request, sections, course.grader, student_module_cache) From 6d650d18251bc0bedb902fcc8404548585f54eb3 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 3 Aug 2012 17:55:25 -0400 Subject: [PATCH 04/51] Got the gradebook working again with the new fast grading method. --- lms/djangoapps/certificates/views.py | 2 ++ lms/djangoapps/courseware/grades.py | 42 ++++++++-------------------- lms/djangoapps/courseware/views.py | 24 ++++++++++------ lms/envs/common.py | 2 +- lms/envs/devplus.py | 4 ++- lms/templates/gradebook.html | 6 ++-- lms/urls.py | 5 +++- 7 files changed, 40 insertions(+), 45 deletions(-) diff --git a/lms/djangoapps/certificates/views.py b/lms/djangoapps/certificates/views.py index 6341133c52..deb78fe3c7 100644 --- a/lms/djangoapps/certificates/views.py +++ b/lms/djangoapps/certificates/views.py @@ -52,6 +52,7 @@ def certificate_request(request): return return_error(survey_response['error']) grade = None + # TODO: (bridger) Update this to use the faster grade instead of grade_sheet student_gradesheet = grades.grade_sheet(request.user) grade = student_gradesheet['grade'] @@ -65,6 +66,7 @@ def certificate_request(request): else: #This is not a POST, we should render the page with the form + # TODO: (bridger) Update this to use the faster grade instead of grade_sheet grade_sheet = grades.grade_sheet(request.user) certificate_state = certificate_state_for_student(request.user, grade_sheet['grade']) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 7cb928e648..d31239eb4f 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -30,6 +30,7 @@ def get_graded_sections(course_descriptor): xmoduledescriptors = [] for module in yield_descriptor_descendents(s): + # TODO: Only include modules that have a score here xmoduledescriptors.append(module) section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : xmoduledescriptors} @@ -73,19 +74,20 @@ def fast_grade(student, request, course_graded_sections, grader, student_module_ scores = [] section_module = get_module(student, request, section_descriptor.location, student_module_cache) - for module in yield_descriptor_descendents(section_module): + # TODO: We may be able to speed this up by only getting a list of children IDs from section_module + # Then, we may not need to instatiate any problems if they are already in the database + for module in yield_module_descendents(section_module): (correct, total) = get_score(student, module, student_module_cache) - graded = module.metadata.get("graded", False) - if correct is None and total is None: continue - + if settings.GENERATE_PROFILE_SCORES: if total > 1: correct = random.randrange(max(total - 2, 1), total + 1) else: correct = total - + + graded = module.metadata.get("graded", False) if not total > 0: #We simply cannot grade a problem that is 12/0, because we might need it as a percentage graded = False @@ -110,20 +112,18 @@ def fast_grade(student, request, course_graded_sections, grader, student_module_ def grade_sheet(student, course, grader, student_module_cache): """ - This pulls a summary of all problems in the course. It returns a dictionary with two datastructures: + This pulls a summary of all problems in the course. + Returns - courseware_summary is a summary of all sections with problems in the course. It is organized as an array of chapters, each containing an array of sections, each containing an array of scores. This contains information for graded and ungraded problems, and is good for displaying a course summary with due dates, etc. - - - grade_summary is the output from the course grader. More information on the format is in the docstring for CourseGrader. - + Arguments: student: A User object for the student to grade course: An XModule containing the course to grade student_module_cache: A StudentModuleCache initialized with all instance_modules for the student """ - totaled_scores = {} chapters = [] for c in course.get_children(): sections = [] @@ -132,30 +132,13 @@ def grade_sheet(student, course, grader, student_module_cache): scores = [] for module in yield_module_descendents(s): (correct, total) = get_score(student, module, student_module_cache) - if correct is None and total is None: continue - if settings.GENERATE_PROFILE_SCORES: - if total > 1: - correct = random.randrange(max(total - 2, 1), total + 1) - else: - correct = total - - if not total > 0: - #We simply cannot grade a problem that is 12/0, because we might need it as a percentage - graded = False - scores.append(Score(correct, total, graded, module.metadata.get('display_name'))) section_total, graded_total = graders.aggregate_scores(scores, s.metadata.get('display_name')) - #Add the graded total to totaled_scores format = s.metadata.get('format', "") - if format and graded_total.possible > 0: - format_scores = totaled_scores.get(format, []) - format_scores.append(graded_total) - totaled_scores[format] = format_scores - sections.append({ 'section': s.metadata.get('display_name'), 'scores': scores, @@ -169,10 +152,7 @@ def grade_sheet(student, course, grader, student_module_cache): 'chapter': c.metadata.get('display_name'), 'sections': sections}) - grade_summary = grader.grade(totaled_scores) - - return {'courseware_summary': chapters, - 'grade_summary': grade_summary} + return chapters def get_score(user, problem, student_module_cache): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index c8939c0b75..fa8245212e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -86,18 +86,20 @@ def gradebook(request, course_id): if 'course_admin' not in user_groups(request.user): raise Http404 course = check_course(course_id) - + + sections, all_descriptors = grades.get_graded_sections(course) + student_objects = User.objects.all()[:100] student_info = [] - + for student in student_objects: - student_module_cache = StudentModuleCache(student, course) - course = get_module(request.user, request, course.location, student_module_cache) + student_module_cache = StudentModuleCache(student, descriptors=all_descriptors) + student_info.append({ 'username': student.username, 'id': student.id, 'email': student.email, - 'grade_info': grades.grade_sheet(student, course, student_module_cache), + 'grade_summary': grades.fast_grade(student, request, sections, course.grader, student_module_cache), 'realname': UserProfile.objects.get(user=student).name }) @@ -123,16 +125,21 @@ def profile(request, course_id, student_id=None): student_module_cache = StudentModuleCache(request.user, course) course_module = get_module(request.user, request, course.location, student_module_cache) + courseware_summary = grades.grade_sheet(student, course_module, course.grader, student_module_cache) + sections, _ = grades.get_graded_sections(course) + grade_summary = grades.fast_grade(request.user, request, sections, course.grader, student_module_cache) + context = {'name': user_info.name, 'username': student.username, 'location': user_info.location, 'language': user_info.language, 'email': student.email, 'course': course, - 'format_url_params': format_url_params, - 'csrf': csrf(request)['csrf_token'] + 'csrf': csrf(request)['csrf_token'], + 'courseware_summary' : courseware_summary, + 'grade_summary' : grade_summary } - context.update(grades.grade_sheet(student, course_module, course.grader, student_module_cache)) + context.update() return render_to_response('profile.html', context) @@ -157,6 +164,7 @@ def render_accordion(request, course, chapter, section): ('toc', toc), ('course_name', course.title), ('course_id', course.id), + #TODO: Do we need format_url_params anymore? What is a better way to create the reversed links? ('format_url_params', format_url_params), ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) return render_to_string('accordion.html', context) diff --git a/lms/envs/common.py b/lms/envs/common.py index 95daa913e8..c5e653ac0e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -41,7 +41,7 @@ PERFSTATS = False MITX_FEATURES = { 'SAMPLE' : False, 'USE_DJANGO_PIPELINE' : True, - 'DISPLAY_HISTOGRAMS_TO_STAFF' : True, + 'DISPLAY_HISTOGRAMS_TO_STAFF' : False, 'REROUTE_ACTIVATION_EMAIL' : False, # nonempty string = address for all activation emails 'DEBUG_LEVEL' : 0, # 0 = lowest level, least verbose, 255 = max level, most verbose diff --git a/lms/envs/devplus.py b/lms/envs/devplus.py index b15322c2c7..bb4524a1ab 100644 --- a/lms/envs/devplus.py +++ b/lms/envs/devplus.py @@ -65,5 +65,7 @@ DEBUG_TOOLBAR_PANELS = ( # Django=1.3.1/1.4 where requests to views get duplicated (your method gets # hit twice). So you can uncomment when you need to diagnose performance # problems, but you shouldn't leave it on. -# 'debug_toolbar.panels.profiling.ProfilingDebugPanel', + 'debug_toolbar.panels.profiling.ProfilingDebugPanel', ) + +#PIPELINE = True diff --git a/lms/templates/gradebook.html b/lms/templates/gradebook.html index bcc0456711..70f23d5a02 100644 --- a/lms/templates/gradebook.html +++ b/lms/templates/gradebook.html @@ -28,7 +28,7 @@ %if len(students) > 0: <% - templateSummary = students[0]['grade_info']['grade_summary'] + templateSummary = students[0]['grade_summary'] %> @@ -58,10 +58,10 @@ %for student in students: - %for section in student['grade_info']['grade_summary']['section_breakdown']: + %for section in student['grade_summary']['section_breakdown']: ${percent_data( section['percent'] )} %endfor - + %endfor
${student['username']}${percent_data( student['grade_info']['grade_summary']['percent'])}${percent_data( student['grade_summary']['percent'])}
diff --git a/lms/urls.py b/lms/urls.py index 8d7460ddc2..10e9e750c9 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -105,7 +105,6 @@ if settings.COURSEWARE_ENABLED: # TODO: These views need to be updated before they work # url(r'^calculate$', 'util.views.calculate'), - # url(r'^gradebook$', 'courseware.views.gradebook'), # TODO: We should probably remove the circuit package. I believe it was only used in the old way of saving wiki circuits for the wiki # url(r'^edit_circuit/(?P[^/]*)$', 'circuit.views.edit_circuit'), # url(r'^save_circuit/(?P[^/]*)$', 'circuit.views.save_circuit'), @@ -139,6 +138,10 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.profile', name="profile"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/profile/(?P[^/]*)/$', 'courseware.views.profile'), + + # For the instructor + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/gradebook$', + 'courseware.views.gradebook'), ) # Multicourse wiki From 7f5246f65323f79336be33992f76044b594a61b2 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Mon, 6 Aug 2012 10:17:53 -0400 Subject: [PATCH 05/51] Removed test view for grading. --- lms/djangoapps/courseware/views.py | 14 +------------- lms/urls.py | 4 ---- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index fa8245212e..4489b08ca0 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -34,19 +34,6 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} - -def test_grading(request, course_id): - course = check_course(course_id) - - sections, all_descriptors = grades.get_graded_sections(course) - student_module_cache = StudentModuleCache(request.user, descriptors=all_descriptors) - - grade_result = grades.fast_grade(request.user, request, sections, course.grader, student_module_cache) - - return HttpResponse("" + json.dumps(grade_result) + "") - - - def user_groups(user): if not user.is_authenticated(): return [] @@ -92,6 +79,7 @@ def gradebook(request, course_id): student_objects = User.objects.all()[:100] student_info = [] + #TODO: Only select students who are in the course for student in student_objects: student_module_cache = StudentModuleCache(student, descriptors=all_descriptors) diff --git a/lms/urls.py b/lms/urls.py index 10e9e750c9..d93eb03873 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -116,10 +116,6 @@ if settings.COURSEWARE_ENABLED: #About the course url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/about$', 'courseware.views.course_about', name="about_course"), - - # TODO: Important. Remove this after testing! - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade$', - 'courseware.views.test_grading', name="info"), #Inside the course url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', From cbab4f1006e77c95b1ce14cb32e680d8d8820fba Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Mon, 6 Aug 2012 11:32:26 -0400 Subject: [PATCH 06/51] Fixed some imports on gradebook.html --- lms/templates/gradebook.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/templates/gradebook.html b/lms/templates/gradebook.html index 70f23d5a02..d4c5461916 100644 --- a/lms/templates/gradebook.html +++ b/lms/templates/gradebook.html @@ -8,6 +8,7 @@ <%block name="headextra"> + <%static:css group='course'/>