diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index 2f9d191083..9920934240 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -170,7 +170,6 @@ def grader_from_conf(conf): weight = subgraderconf.pop("weight", 0) try: if 'min_count' in subgraderconf: - #This is an AssignmentFormatGrader subgrader_class = AssignmentFormatGrader else: raise ValueError("Configuration has no appropriate grader class.") @@ -344,28 +343,28 @@ class AssignmentFormatGrader(CourseGrader): self.starting_index = starting_index self.hide_average = hide_average + def total_with_drops(self, breakdown): + """ + Calculates total score for a section while dropping lowest scores + """ + # Create an array of tuples with (index, mark), sorted by mark['percent'] descending + sorted_breakdown = sorted(enumerate(breakdown), key=lambda x: -x[1]['percent']) + + # A list of the indices of the dropped scores + dropped_indices = [] + if self.drop_count > 0: + dropped_indices = [x[0] for x in sorted_breakdown[-self.drop_count:]] + aggregate_score = 0 + for index, mark in enumerate(breakdown): + if index not in dropped_indices: + aggregate_score += mark['percent'] + + if len(breakdown) - self.drop_count > 0: + aggregate_score /= len(breakdown) - self.drop_count + + return aggregate_score, dropped_indices + def grade(self, grade_sheet, generate_random_scores=False): - def total_with_drops(breakdown, drop_count): - """ - Calculates total score for a section while dropping lowest scores - """ - # Create an array of tuples with (index, mark), sorted by mark['percent'] descending - sorted_breakdown = sorted(enumerate(breakdown), key=lambda x: -x[1]['percent']) - - # A list of the indices of the dropped scores - dropped_indices = [] - if drop_count > 0: - dropped_indices = [x[0] for x in sorted_breakdown[-drop_count:]] - aggregate_score = 0 - for index, mark in enumerate(breakdown): - if index not in dropped_indices: - aggregate_score += mark['percent'] - - if len(breakdown) - drop_count > 0: - aggregate_score /= len(breakdown) - drop_count - - return aggregate_score, dropped_indices - scores = grade_sheet.get(self.type, {}).values() breakdown = [] for i in range(max(self.min_count, len(scores))): @@ -405,7 +404,7 @@ class AssignmentFormatGrader(CourseGrader): breakdown.append({'percent': percentage, 'label': short_label, 'detail': summary, 'category': self.category}) - total_percent, dropped_indices = total_with_drops(breakdown, self.drop_count) + total_percent, dropped_indices = self.total_with_drops(breakdown) for dropped_index in dropped_indices: breakdown[dropped_index]['mark'] = { diff --git a/lms/djangoapps/grades/context.py b/lms/djangoapps/grades/context.py index 3b643a6133..0c49f959f9 100644 --- a/lms/djangoapps/grades/context.py +++ b/lms/djangoapps/grades/context.py @@ -5,18 +5,19 @@ from collections import OrderedDict from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache +from .course_grade import CourseGrade from .scores import possibly_scored -def grading_context_for_course(course_key): +def grading_context_for_course(course): """ Same as grading_context, but takes in a course key. """ - course_structure = get_course_in_cache(course_key) - return grading_context(course_structure) + course_structure = get_course_in_cache(course.id) + return grading_context(course, course_structure) -def grading_context(course_structure): +def grading_context(course, course_structure): """ This returns a dictionary with keys necessary for quickly grading a student. @@ -36,7 +37,7 @@ def grading_context(course_structure): the descriptor tree again. """ - all_graded_blocks = [] + count_all_graded_blocks = 0 all_graded_subsections_by_type = OrderedDict() for chapter_key in course_structure.get_children(course_structure.root_block_usage_key): @@ -64,9 +65,10 @@ def grading_context(course_structure): if subsection_format not in all_graded_subsections_by_type: all_graded_subsections_by_type[subsection_format] = [] all_graded_subsections_by_type[subsection_format].append(subsection_info) - all_graded_blocks.extend(scored_descendants_of_subsection) + count_all_graded_blocks += len(scored_descendants_of_subsection) return { 'all_graded_subsections_by_type': all_graded_subsections_by_type, - 'all_graded_blocks': all_graded_blocks, + 'count_all_graded_blocks': count_all_graded_blocks, + 'subsection_type_graders': CourseGrade.get_subsection_type_graders(course) } diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index a7d954a5ab..21524bb54f 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -32,14 +32,14 @@ class CourseData(object): if self._course: self._course_key = self._course.id else: - structure = self._effective_structure + structure = self.effective_structure self._course_key = structure.root_block_usage_key.course_key return self._course_key @property def location(self): if not self._location: - structure = self._effective_structure + structure = self.effective_structure if structure: self._location = structure.root_block_usage_key elif self._course: @@ -72,7 +72,7 @@ class CourseData(object): @property def grading_policy_hash(self): - structure = self._effective_structure + structure = self.effective_structure if structure: return structure.get_transformer_block_field( structure.root_block_usage_key, @@ -84,14 +84,14 @@ class CourseData(object): @property def version(self): - structure = self._effective_structure + structure = self.effective_structure course_block = structure[self.location] if structure else self.course return getattr(course_block, 'course_version', None) @property def edited_on(self): # get course block from structure only; subtree_edited_on field on modulestore's course block isn't optimized. - structure = self._effective_structure + structure = self.effective_structure if structure: course_block = structure[self.location] return getattr(course_block, 'subtree_edited_on', None) @@ -100,7 +100,7 @@ class CourseData(object): return u'Course: course_key: {}'.format(self.course_key) def full_string(self): - if self._effective_structure: + if self.effective_structure: return u'Course: course_key: {}, version: {}, edited_on: {}, grading_policy: {}'.format( self.course_key, self.version, self.edited_on, self.grading_policy_hash, ) @@ -108,5 +108,5 @@ class CourseData(object): return u'Course: course_key: {}, empty course structure'.format(self.course_key) @property - def _effective_structure(self): + def effective_structure(self): return self._structure or self._collected_block_structure diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index 58e47963a1..c50505f2b9 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -10,6 +10,7 @@ from lazy import lazy from ccx_keys.locator import CCXLocator from xmodule import block_metadata_utils +from .config import assume_zero_if_absent from .subsection_grade import ZeroSubsectionGrade from .subsection_grade_factory import SubsectionGradeFactory @@ -46,17 +47,14 @@ class CourseGradeBase(object): def subsection_grade(self, subsection_key): """ - Returns the subsection grade for given subsection usage key. - Raises KeyError if the user doesn't have access to that subsection. - """ - return self._get_subsection_grade(self.course_data.structure[subsection_key]) + Returns the subsection grade for the given subsection usage key. - @abstractmethod - def assignment_average(self, assignment_type): + Note: does NOT check whether the user has access to the subsection. + Assumes that if a grade exists, the user has access to it. If the + grade doesn't exist then either the user does not have access to + it or hasn't attempted any problems in the subsection. """ - Returns the average of all assignments of the given assignment type. - """ - raise NotImplementedError + return self._get_subsection_grade(self.course_data.effective_structure[subsection_key]) @lazy def graded_subsections_by_format(self): @@ -148,7 +146,7 @@ class CourseGradeBase(object): """ Returns the result from the course grader. """ - course = self._prep_course_for_grading() + course = self._prep_course_for_grading(self.course_data.course) return course.grader.grade( self.graded_subsections_by_format, generate_random_scores=settings.GENERATE_PROFILE_SCORES, @@ -166,7 +164,21 @@ class CourseGradeBase(object): grade_summary['grade'] = self.letter_grade return grade_summary - def _prep_course_for_grading(self): + @classmethod + def get_subsection_type_graders(cls, course): + """ + Returns a dictionary mapping subsection types to their + corresponding configured graders, per grading policy. + """ + course = cls._prep_course_for_grading(course) + return { + subsection_type: subsection_type_grader + for (subsection_type_grader, subsection_type, _) + in course.grader.subgraders + } + + @classmethod + def _prep_course_for_grading(cls, course): """ Make sure any overrides to the grading policy are used. This is most relevant for CCX courses. @@ -176,8 +188,7 @@ class CourseGradeBase(object): this will no longer be needed - since BlockStructure correctly retrieves/uses all field overrides. """ - course = self.course_data.course - if isinstance(self.course_data.course_key, CCXLocator): + if isinstance(course.id, CCXLocator): # clean out any field values that may have been set from the # parent course of the CCX course. course._field_data_cache = {} # pylint: disable=protected-access @@ -221,9 +232,6 @@ class ZeroCourseGrade(CourseGradeBase): Course Grade class for Zero-value grades when no problems were attempted in the course. """ - def assignment_average(self, assignment_type): - return 0.0 - def _get_subsection_grade(self, subsection): return ZeroSubsectionGrade(subsection, self.course_data) @@ -259,15 +267,15 @@ class CourseGrade(CourseGradeBase): Returns whether any of the subsections in this course have been attempted by the student. """ + if assume_zero_if_absent(self.course_data.course_key): + return True + for chapter in self.chapter_grades.itervalues(): for subsection_grade in chapter['sections']: if subsection_grade.all_total.first_attempted: return True return False - def assignment_average(self, assignment_type): - return self.grader_result['grade_breakdown'].get(assignment_type, {}).get('percent') - def _get_subsection_grade(self, subsection): if self.force_update_subsections: return self._subsection_grade_factory.update(subsection) diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index 44f1418aee..d818e3b2be 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -85,7 +85,7 @@ class CourseGradeFactory(object): If an error occurred, course_grade will be None and err_msg will be an exception message. If there was no error, err_msg is an empty string. """ - # Pre-fetch the collected course_structure so: + # Pre-fetch the collected course_structure (in _iter_grade_result) so: # 1. Correctness: the same version of the course is used to # compute the grade for all students. # 2. Optimization: the collected course_structure is not diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 1c689da928..a45efc29d6 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -63,6 +63,13 @@ class SubsectionGradeBase(object): """ raise NotImplementedError + @property + def percent_graded(self): + """ + Returns the percent score of the graded problems in this subsection. + """ + raise NotImplementedError + class ZeroSubsectionGrade(SubsectionGradeBase): """ @@ -77,6 +84,10 @@ class ZeroSubsectionGrade(SubsectionGradeBase): def attempted_graded(self): return False + @property + def percent_graded(self): + return 0.0 + @property def all_total(self): return self._aggregate_scores[0] @@ -128,6 +139,10 @@ class NonZeroSubsectionGrade(SubsectionGradeBase): def attempted_graded(self): return self.graded_total.first_attempted is not None + @property + def percent_graded(self): + return self.graded_total.earned / self.graded_total.possible + @staticmethod def _compute_block_score( block_key, @@ -157,7 +172,7 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade): """ Class for Subsection grades that are read from the database. """ - def __init__(self, subsection, model, course_structure, submissions_scores, csm_scores): + def __init__(self, subsection, model, factory): all_total = AggregatedScore( tw_earned=model.earned_all, tw_possible=model.possible_all, @@ -174,9 +189,7 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade): # save these for later since we compute problem_scores lazily self.model = model - self.course_structure = course_structure - self.submissions_scores = submissions_scores - self.csm_scores = csm_scores + self.factory = factory super(ReadSubsectionGrade, self).__init__(subsection, all_total, graded_total, override) @@ -185,7 +198,11 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade): problem_scores = OrderedDict() for block in self.model.visible_blocks.blocks: problem_score = self._compute_block_score( - block.locator, self.course_structure, self.submissions_scores, self.csm_scores, block, + block.locator, + self.factory.course_data.structure, + self.factory._submissions_scores, + self.factory._csm_scores, + block, ) if problem_score: problem_scores[block.locator] = problem_score diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index dec0f4fbae..dfbd21ec11 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -80,9 +80,7 @@ class SubsectionGradeFactory(object): except PersistentSubsectionGrade.DoesNotExist: pass else: - orig_subsection_grade = ReadSubsectionGrade( - subsection, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores, - ) + orig_subsection_grade = ReadSubsectionGrade(subsection, grade_model, self) if not is_score_higher_or_equal( orig_subsection_grade.graded_total.earned, orig_subsection_grade.graded_total.possible, @@ -125,9 +123,7 @@ class SubsectionGradeFactory(object): saved_subsection_grades = self._get_bulk_cached_subsection_grades() grade = saved_subsection_grades.get(subsection.location) if grade: - return ReadSubsectionGrade( - subsection, grade, self.course_data.structure, self._submissions_scores, self._csm_scores, - ) + return ReadSubsectionGrade(subsection, grade, self) def _get_bulk_cached_subsection_grades(self): """ diff --git a/lms/djangoapps/grades/tests/base.py b/lms/djangoapps/grades/tests/base.py index 47d44e3dee..0d805b944d 100644 --- a/lms/djangoapps/grades/tests/base.py +++ b/lms/djangoapps/grades/tests/base.py @@ -6,6 +6,7 @@ from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from ..course_data import CourseData from ..subsection_grade_factory import SubsectionGradeFactory @@ -75,6 +76,7 @@ class GradeTestBase(SharedModuleStoreTestCase): self.client.login(username=self.request.user.username, password="test") self._set_grading_policy() self.course_structure = get_course_blocks(self.request.user, self.course.location) + self.course_data = CourseData(self.request.user, structure=self.course_structure) self.subsection_grade_factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure) CourseEnrollment.enroll(self.request.user, self.course.id) @@ -91,6 +93,13 @@ class GradeTestBase(SharedModuleStoreTestCase): "short_label": "HW", "weight": 1.0, }, + { + "type": "NoCredit", + "min_count": 0, + "drop_count": 0, + "short_label": "NC", + "weight": 0.0, + }, ], "GRADE_CUTOFFS": { "Pass": passing, diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index ad101ba7e0..577837ab11 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -97,19 +97,19 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(29), mock_get_score(1, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(4): + with self.assertNumQueries(2): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - with self.assertNumQueries(6), mock_get_score(1, 4): + with self.assertNumQueries(4), mock_get_score(1, 4): grade_factory.update(self.request.user, self.course, force_update_subsections=False) - with self.assertNumQueries(4): + with self.assertNumQueries(2): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 with self.assertNumQueries(12), mock_get_score(2, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(4): + with self.assertNumQueries(2): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @@ -124,6 +124,35 @@ class TestCourseGradeFactory(GradeTestBase): else: self.assertIsNone(course_grade) + def test_read_optimization(self): + grade_factory = CourseGradeFactory() + with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mocked_course_blocks: + mocked_course_blocks.return_value = self.course_structure + with mock_get_score(1, 2): + grade_factory.update(self.request.user, self.course, force_update_subsections=True) + self.assertEquals(mocked_course_blocks.call_count, 1) + + with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mocked_course_blocks: + with patch('lms.djangoapps.grades.subsection_grade.get_score') as mocked_get_score: + course_grade = grade_factory.read(self.request.user, self.course) + self.assertEqual(course_grade.percent, 0.5) # make sure it's not a zero-valued course grade + self.assertFalse(mocked_get_score.called) # no calls to CSM/submissions tables + self.assertFalse(mocked_course_blocks.called) # no user-specific transformer calculation + + def test_subsection_grade(self): + grade_factory = CourseGradeFactory() + with mock_get_score(1, 2): + grade_factory.update(self.request.user, self.course, force_update_subsections=True) + course_grade = grade_factory.read(self.request.user, course_structure=self.course_structure) + subsection_grade = course_grade.subsection_grade(self.sequence.location) + self.assertEqual(subsection_grade.percent_graded, 0.5) + + def test_subsection_type_graders(self): + graders = CourseGrade.get_subsection_type_graders(self.course) + self.assertEqual(len(graders), 2) + self.assertEqual(graders["Homework"].type, "Homework") + self.assertEqual(graders["NoCredit"].min_count, 0) + def test_create_zero_subs_grade_for_nonzero_course_grade(self): subsection = self.course_structure[self.sequence.location] with mock_get_score(1, 2): @@ -158,6 +187,11 @@ class TestCourseGradeFactory(GradeTestBase): 'category': 'Homework', 'percent': 0.25, 'detail': 'Homework = 25.00% of a possible 100.00%', + }, + 'NoCredit': { + 'category': 'NoCredit', + 'percent': 0.0, + 'detail': 'NoCredit = 0.00% of a possible 0.00%', } }, 'percent': 0.25, @@ -181,6 +215,13 @@ class TestCourseGradeFactory(GradeTestBase): 'percent': 0.25, 'prominent': True }, + { + 'category': 'NoCredit', + 'detail': u'NoCredit Average = 0%', + 'label': u'NC Avg', + 'percent': 0, + 'prominent': True + }, ] } self.assertEqual(expected_summary, actual_summary) diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py index a9390f3e4f..88311db9e4 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -15,6 +15,7 @@ class SubsectionGradeTest(GradeTestBase): self.subsection_grade_factory._csm_scores, ) self.assertEqual(PersistentSubsectionGrade.objects.count(), 0) + self.assertEqual(created_grade.percent_graded, 0.5) # save to db, and verify object is in database created_grade.update_or_create_model(self.request.user) @@ -28,11 +29,10 @@ class SubsectionGradeTest(GradeTestBase): read_grade = ReadSubsectionGrade( self.sequence, saved_model, - self.course_structure, - self.subsection_grade_factory._submissions_scores, - self.subsection_grade_factory._csm_scores, + self.subsection_grade_factory ) self.assertEqual(created_grade.url_name, read_grade.url_name) read_grade.all_total.first_attempted = created_grade.all_total.first_attempted = None self.assertEqual(created_grade.all_total, read_grade.all_total) + self.assertEqual(created_grade.percent_graded, 0.5) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 1cf7085f53..c8dc4f6773 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,10 +164,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 25, True), - (ModuleStoreEnum.Type.mongo, 1, 25, False), - (ModuleStoreEnum.Type.split, 3, 25, True), - (ModuleStoreEnum.Type.split, 3, 25, False), + (ModuleStoreEnum.Type.mongo, 1, 23, True), + (ModuleStoreEnum.Type.mongo, 1, 23, False), + (ModuleStoreEnum.Type.split, 3, 23, True), + (ModuleStoreEnum.Type.split, 3, 23, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 25), - (ModuleStoreEnum.Type.split, 3, 25), + (ModuleStoreEnum.Type.mongo, 1, 23), + (ModuleStoreEnum.Type.split, 3, 23), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -240,8 +240,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 26), - (ModuleStoreEnum.Type.split, 3, 26), + (ModuleStoreEnum.Type.mongo, 1, 24), + (ModuleStoreEnum.Type.split, 3, 24), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 7123fc46a1..fb54cc9416 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -518,7 +518,7 @@ def dump_grading_context(course): msg += hbar msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string() - gcontext = grading_context_for_course(course.id) + gcontext = grading_context_for_course(course) msg += "graded sections:\n" msg += '%s\n' % gcontext['all_graded_subsections_by_type'].keys() @@ -541,6 +541,6 @@ def dump_grading_context(course): msg += " %s (format=%s, Assignment=%s%s)\n"\ % (sdesc.display_name, frmat, aname, notes) msg += "all graded blocks:\n" - msg += "length=%d\n" % len(gcontext['all_graded_blocks']) + msg += "length=%d\n" % gcontext['count_all_graded_blocks'] msg = '
%s' % msg.replace('<', '<') return msg diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 7c215d7961..c7fee1bf57 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -103,7 +103,7 @@ class _CourseGradeReportContext(object): Returns an OrderedDict that maps an assignment type to a dict of subsection-headers and average-header. """ - grading_cxt = grading_context(self.course_structure) + grading_cxt = grading_context(self.course, self.course_structure) graded_assignments_map = OrderedDict() for assignment_type_name, subsection_infos in grading_cxt['all_graded_subsections_by_type'].iteritems(): graded_subsections_map = OrderedDict() @@ -127,7 +127,8 @@ class _CourseGradeReportContext(object): graded_assignments_map[assignment_type_name] = { 'subsection_headers': graded_subsections_map, 'average_header': average_header, - 'separate_subsection_avg_headers': separate_subsection_avg_headers + 'separate_subsection_avg_headers': separate_subsection_avg_headers, + 'grader': grading_cxt['subsection_type_graders'].get(assignment_type_name), } return graded_assignments_map @@ -297,29 +298,56 @@ class CourseGradeReport(object): users = users.select_related('profile__allow_certificate') return grouper(users) - def _user_grade_results(self, course_grade, context): + def _user_grades(self, course_grade, context): """ Returns a list of grade results for the given course_grade corresponding to the headers for this report. """ grade_results = [] for assignment_type, assignment_info in context.graded_assignments.iteritems(): - for subsection_location in assignment_info['subsection_headers']: - try: - subsection_grade = course_grade.subsection_grade(subsection_location) - except KeyError: - grade_result = u'Not Available' - else: - if subsection_grade.attempted_graded: - grade_result = subsection_grade.graded_total.earned / subsection_grade.graded_total.possible - else: - grade_result = u'Not Attempted' - grade_results.append([grade_result]) - if assignment_info['separate_subsection_avg_headers']: - assignment_average = course_grade.assignment_average(assignment_type) + + subsection_grades, subsection_grades_results = self._user_subsection_grades( + course_grade, + assignment_info['subsection_headers'], + ) + grade_results.extend(subsection_grades_results) + + assignment_average = self._user_assignment_average(course_grade, subsection_grades, assignment_info) + if assignment_average is not None: grade_results.append([assignment_average]) + return [course_grade.percent] + _flatten(grade_results) + def _user_subsection_grades(self, course_grade, subsection_headers): + """ + Returns a list of grade results for the given course_grade corresponding + to the headers for this report. + """ + subsection_grades = [] + grade_results = [] + for subsection_location in subsection_headers: + subsection_grade = course_grade.subsection_grade(subsection_location) + if subsection_grade.attempted_graded: + grade_result = subsection_grade.percent_graded + else: + grade_result = u'Not Attempted' + grade_results.append([grade_result]) + subsection_grades.append(subsection_grade) + return subsection_grades, grade_results + + def _user_assignment_average(self, course_grade, subsection_grades, assignment_info): + if assignment_info['separate_subsection_avg_headers']: + if assignment_info['grader']: + if course_grade.attempted: + subsection_breakdown = [ + {'percent': subsection_grade.percent_graded} + for subsection_grade in subsection_grades + ] + assignment_average, _ = assignment_info['grader'].total_with_drops(subsection_breakdown) + else: + assignment_average = 0.0 + return assignment_average + def _user_cohort_group_names(self, user, context): """ Returns a list of names of cohort groups in which the given user @@ -411,7 +439,7 @@ class CourseGradeReport(object): else: success_rows.append( [user.id, user.email, user.username] + - self._user_grade_results(course_grade, context) + + self._user_grades(course_grade, context) + self._user_cohort_group_names(user, context) + self._user_experiment_group_names(user, context) + self._user_team_names(user, bulk_context.teams) + @@ -440,7 +468,8 @@ class ProblemGradeReport(object): # as the keys. It is structured in this way to keep the values related. header_row = OrderedDict([('id', 'Student ID'), ('email', 'Email'), ('username', 'Username')]) - graded_scorable_blocks = cls._graded_scorable_blocks_to_header(course_id) + course = get_course_by_id(course_id) + graded_scorable_blocks = cls._graded_scorable_blocks_to_header(course) # Just generate the static fields for now. rows = [list(header_row.values()) + ['Enrollment Status', 'Grade'] + _flatten(graded_scorable_blocks.values())] @@ -451,7 +480,6 @@ class ProblemGradeReport(object): # whether each user is currently enrolled in the course. CourseEnrollment.bulk_fetch_enrollment_states(enrolled_students, course_id) - course = get_course_by_id(course_id) for student, course_grade, error in CourseGradeFactory().iter(enrolled_students, course): student_fields = [getattr(student, field_name) for field_name in header_row] task_progress.attempted += 1 @@ -495,13 +523,13 @@ class ProblemGradeReport(object): return task_progress.update_task_state(extra_meta={'step': 'Uploading CSV'}) @classmethod - def _graded_scorable_blocks_to_header(cls, course_key): + def _graded_scorable_blocks_to_header(cls, course): """ Returns an OrderedDict that maps a scorable block's id to its headers in the final report. """ scorable_blocks_map = OrderedDict() - grading_context = grading_context_for_course(course_key) + grading_context = grading_context_for_course(course) for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].iteritems(): for subsection_index, subsection_info in enumerate(subsection_infos, start=1): for scorable_block in subsection_info['scored_descendants']: diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 32409b11bf..1c92e54a50 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1707,6 +1707,7 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): ) +@ddt.ddt @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage) class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): """ @@ -1782,7 +1783,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): u'Username': self.student.username, u'Grade': '0.13', u'Homework 1: Subsection': '0.5', - u'Homework 2: Hidden': u'Not Available', + u'Homework 2: Hidden': u'Not Attempted', u'Homework 3: Unattempted': u'Not Attempted', u'Homework 4: Empty': u'Not Attempted', u'Homework (Avg)': '0.125', @@ -1791,22 +1792,16 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): ignore_other_columns=True, ) - def test_fast_generation_zero_grade(self): + @ddt.data(True, False) + def test_fast_generation(self, create_non_zero_grade): + if create_non_zero_grade: + self.submit_student_answer(self.student.username, u'Problem1', ['Option 1']) with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): - with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader: + with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mock_course_blocks: with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: CourseGradeReport.generate(None, None, self.course.id, None, 'graded') - self.assertFalse(mock_grader.called) self.assertFalse(mock_get_score.called) - - def test_slow_generation_nonzero_grade(self): - self.submit_student_answer(self.student.username, u'Problem1', ['Option 1']) - with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): - with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader: - with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: - CourseGradeReport.generate(None, None, self.course.id, None, 'graded') - self.assertTrue(mock_grader.called) - self.assertTrue(mock_get_score.called) + self.assertFalse(mock_course_blocks.called) @ddt.ddt