From 33428519aa7a99932d71261b052a40adc19cf037 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 4 Oct 2017 09:30:07 -0400 Subject: [PATCH] Have SubsectionGrade compute problem_scores lazily --- lms/djangoapps/grades/subsection_grade.py | 213 +++++++++--------- .../grades/subsection_grade_factory.py | 12 +- .../grades/tests/test_course_grade_factory.py | 4 +- .../grades/tests/test_subsection_grade.py | 6 +- .../instructor_task/tasks_helper/grades.py | 4 +- 5 files changed, 115 insertions(+), 124 deletions(-) diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 6f90a36236..1c689da928 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -1,6 +1,7 @@ """ SubsectionGrade Class """ +from abc import ABCMeta from collections import OrderedDict from logging import getLogger @@ -17,8 +18,10 @@ log = getLogger(__name__) class SubsectionGradeBase(object): """ - Class for Subsection Grades. + Abstract base class for Subsection Grades. """ + __metaclass__ = ABCMeta + def __init__(self, subsection): self.location = subsection.location self.display_name = block_metadata_utils.display_name_with_default_escaped(subsection) @@ -108,100 +111,23 @@ class ZeroSubsectionGrade(SubsectionGradeBase): return locations -class SubsectionGrade(SubsectionGradeBase): +class NonZeroSubsectionGrade(SubsectionGradeBase): """ - Class for Subsection Grades. + Abstract base class for Subsection Grades with + possibly NonZero values. """ - def __init__(self, subsection, problem_scores, all_total, graded_total, override=None): - super(SubsectionGrade, self).__init__(subsection) - self.problem_scores = problem_scores + __metaclass__ = ABCMeta + + def __init__(self, subsection, all_total, graded_total, override=None): + super(NonZeroSubsectionGrade, self).__init__(subsection) self.all_total = all_total self.graded_total = graded_total self.override = override - @classmethod - def create(cls, subsection, course_structure, submissions_scores, csm_scores): - """ - Compute and create the subsection grade. - """ - problem_scores = OrderedDict() - for block_key in course_structure.post_order_traversal( - filter_func=possibly_scored, - start_node=subsection.location, - ): - problem_score = cls._compute_block_score(block_key, course_structure, submissions_scores, csm_scores) - if problem_score: - problem_scores[block_key] = problem_score - all_total, graded_total = graders.aggregate_scores(problem_scores.values()) - - return cls(subsection, problem_scores, all_total, graded_total) - - @classmethod - def read(cls, subsection, model, course_structure, submissions_scores, csm_scores): - """ - Read the subsection grade from the persisted model. - """ - problem_scores = OrderedDict() - for block in model.visible_blocks.blocks: - problem_score = cls._compute_block_score( - block.locator, course_structure, submissions_scores, csm_scores, block, - ) - if problem_score: - problem_scores[block.locator] = problem_score - - all_total = AggregatedScore( - tw_earned=model.earned_all, - tw_possible=model.possible_all, - graded=False, - first_attempted=model.first_attempted, - ) - graded_total = AggregatedScore( - tw_earned=model.earned_graded, - tw_possible=model.possible_graded, - graded=True, - first_attempted=model.first_attempted, - ) - override = model.override if hasattr(model, 'override') else None - return cls(subsection, problem_scores, all_total, graded_total, override) - - @classmethod - def bulk_create_models(cls, student, subsection_grades, course_key): - """ - Saves the subsection grade in a persisted model. - """ - params = [ - subsection_grade._persisted_model_params(student) # pylint: disable=protected-access - for subsection_grade in subsection_grades - if subsection_grade - if subsection_grade._should_persist_per_attempted() # pylint: disable=protected-access - ] - return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) - - def update_or_create_model(self, student, score_deleted=False): - """ - Saves or updates the subsection grade in a persisted model. - """ - if self._should_persist_per_attempted(score_deleted): - self._log_event(log.debug, u"update_or_create_model", student) - return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) - @property def attempted_graded(self): return self.graded_total.first_attempted is not None - def _should_persist_per_attempted(self, score_deleted=False): - """ - Returns whether the SubsectionGrade's model should be - persisted based on settings and attempted status. - - If the learner's score was just deleted, they will have - no attempts but the grade should still be persisted. - """ - return ( - self.all_total.first_attempted is not None or - score_deleted - ) - @staticmethod def _compute_block_score( block_key, @@ -210,10 +136,6 @@ class SubsectionGrade(SubsectionGradeBase): csm_scores, persisted_block=None, ): - """ - Compute score for the given block. If persisted_values - is provided, it is used for possible and weight. - """ try: block = course_structure[block_key] except KeyError: @@ -230,6 +152,97 @@ class SubsectionGrade(SubsectionGradeBase): block, ) + +class ReadSubsectionGrade(NonZeroSubsectionGrade): + """ + Class for Subsection grades that are read from the database. + """ + def __init__(self, subsection, model, course_structure, submissions_scores, csm_scores): + all_total = AggregatedScore( + tw_earned=model.earned_all, + tw_possible=model.possible_all, + graded=False, + first_attempted=model.first_attempted, + ) + graded_total = AggregatedScore( + tw_earned=model.earned_graded, + tw_possible=model.possible_graded, + graded=True, + first_attempted=model.first_attempted, + ) + override = model.override if hasattr(model, 'override') else None + + # 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 + + super(ReadSubsectionGrade, self).__init__(subsection, all_total, graded_total, override) + + @lazy + def problem_scores(self): + 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, + ) + if problem_score: + problem_scores[block.locator] = problem_score + return problem_scores + + +class CreateSubsectionGrade(NonZeroSubsectionGrade): + """ + Class for Subsection grades that are newly created or updated. + """ + def __init__(self, subsection, course_structure, submissions_scores, csm_scores): + self.problem_scores = OrderedDict() + for block_key in course_structure.post_order_traversal( + filter_func=possibly_scored, + start_node=subsection.location, + ): + problem_score = self._compute_block_score(block_key, course_structure, submissions_scores, csm_scores) + if problem_score: + self.problem_scores[block_key] = problem_score + + all_total, graded_total = graders.aggregate_scores(self.problem_scores.values()) + + super(CreateSubsectionGrade, self).__init__(subsection, all_total, graded_total) + + def update_or_create_model(self, student, score_deleted=False): + """ + Saves or updates the subsection grade in a persisted model. + """ + if self._should_persist_per_attempted(score_deleted): + return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) + + @classmethod + def bulk_create_models(cls, student, subsection_grades, course_key): + """ + Saves the subsection grade in a persisted model. + """ + params = [ + subsection_grade._persisted_model_params(student) # pylint: disable=protected-access + for subsection_grade in subsection_grades + if subsection_grade + if subsection_grade._should_persist_per_attempted() # pylint: disable=protected-access + ] + return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) + + def _should_persist_per_attempted(self, score_deleted=False): + """ + Returns whether the SubsectionGrade's model should be + persisted based on settings and attempted status. + + If the learner's score was just deleted, they will have + no attempts but the grade should still be persisted. + """ + return ( + self.all_total.first_attempted is not None or + score_deleted + ) + def _persisted_model_params(self, student): """ Returns the parameters for creating/updating the @@ -258,25 +271,3 @@ class SubsectionGrade(SubsectionGradeBase): for location, score in self.problem_scores.iteritems() ] - - def _log_event(self, log_func, log_statement, student): - """ - Logs the given statement, for this instance. - """ - log_func( - u"Grades: SG.{}, subsection: {}, course: {}, " - u"version: {}, edit: {}, user: {}," - u"total: {}/{}, graded: {}/{}, show_correctness: {}".format( - log_statement, - self.location, - self.location.course_key, - self.course_version, - self.subtree_edited_timestamp, - student.id, - self.all_total.earned, - self.all_total.possible, - self.graded_total.earned, - self.graded_total.possible, - self.show_correctness, - ) - ) diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index 08cfe01a22..dec0f4fbae 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -12,7 +12,7 @@ from student.models import anonymous_id_for_user from submissions import api as submissions_api from .course_data import CourseData -from .subsection_grade import SubsectionGrade, ZeroSubsectionGrade +from .subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade, ZeroSubsectionGrade log = getLogger(__name__) @@ -43,7 +43,7 @@ class SubsectionGradeFactory(object): if assume_zero_if_absent(self.course_data.course_key): subsection_grade = ZeroSubsectionGrade(subsection, self.course_data) else: - subsection_grade = SubsectionGrade.create( + subsection_grade = CreateSubsectionGrade( subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) if should_persist_grades(self.course_data.course_key): @@ -58,7 +58,7 @@ class SubsectionGradeFactory(object): """ Bulk creates all the unsaved subsection_grades to this point. """ - SubsectionGrade.bulk_create_models( + CreateSubsectionGrade.bulk_create_models( self.student, self._unsaved_subsection_grades.values(), self.course_data.course_key ) self._unsaved_subsection_grades.clear() @@ -69,7 +69,7 @@ class SubsectionGradeFactory(object): """ self._log_event(log.debug, u"update, subsection: {}".format(subsection.location), subsection) - calculated_grade = SubsectionGrade.create( + calculated_grade = CreateSubsectionGrade( subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) @@ -80,7 +80,7 @@ class SubsectionGradeFactory(object): except PersistentSubsectionGrade.DoesNotExist: pass else: - orig_subsection_grade = SubsectionGrade.read( + orig_subsection_grade = ReadSubsectionGrade( subsection, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores, ) if not is_score_higher_or_equal( @@ -125,7 +125,7 @@ class SubsectionGradeFactory(object): saved_subsection_grades = self._get_bulk_cached_subsection_grades() grade = saved_subsection_grades.get(subsection.location) if grade: - return SubsectionGrade.read( + return ReadSubsectionGrade( subsection, grade, self.course_data.structure, self._submissions_scores, self._csm_scores, ) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index c0337a56df..ad101ba7e0 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -14,7 +14,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle from ..course_grade import CourseGrade, ZeroCourseGrade from ..course_grade_factory import CourseGradeFactory -from ..subsection_grade import SubsectionGrade, ZeroSubsectionGrade +from ..subsection_grade import ReadSubsectionGrade, ZeroSubsectionGrade from .base import GradeTestBase from .utils import mock_get_score @@ -131,7 +131,7 @@ class TestCourseGradeFactory(GradeTestBase): course_grade = CourseGradeFactory().update(self.request.user, self.course) subsection1_grade = course_grade.subsection_grades[self.sequence.location] subsection2_grade = course_grade.subsection_grades[self.sequence2.location] - self.assertIsInstance(subsection1_grade, SubsectionGrade) + self.assertIsInstance(subsection1_grade, ReadSubsectionGrade) self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade) @ddt.data(True, False) diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py index a3187add45..a9390f3e4f 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -1,5 +1,5 @@ from ..models import PersistentSubsectionGrade -from ..subsection_grade import SubsectionGrade +from ..subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade from .utils import mock_get_score from .base import GradeTestBase @@ -8,7 +8,7 @@ class SubsectionGradeTest(GradeTestBase): def test_create_and_read(self): with mock_get_score(1, 2): # Create a grade that *isn't* saved to the database - created_grade = SubsectionGrade.create( + created_grade = CreateSubsectionGrade( self.sequence, self.course_structure, self.subsection_grade_factory._submissions_scores, @@ -25,7 +25,7 @@ class SubsectionGradeTest(GradeTestBase): user_id=self.request.user.id, usage_key=self.sequence.location, ) - read_grade = SubsectionGrade.read( + read_grade = ReadSubsectionGrade( self.sequence, saved_model, self.course_structure, diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 2876f4c06a..7c215d7961 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -222,7 +222,7 @@ class CourseGradeReport(object): Returns a list of all applicable column headers for this grade report. """ return ( - ["Student ID", "Email", "Username", "Grade"] + + ["Student ID", "Email", "Username"] + self._grades_header(context) + (['Cohort Name'] if context.cohorts_enabled else []) + [u'Experiment Group ({})'.format(partition.name) for partition in context.course_experiments] + @@ -278,7 +278,7 @@ class CourseGradeReport(object): Returns the applicable grades-related headers for this report. """ graded_assignments = context.graded_assignments - grades_header = [] + grades_header = ["Grade"] for assignment_info in graded_assignments.itervalues(): if assignment_info['separate_subsection_avg_headers']: grades_header.extend(assignment_info['subsection_headers'].itervalues())