diff --git a/README.rst b/README.rst index 1881165..e097d3e 100644 --- a/README.rst +++ b/README.rst @@ -32,12 +32,12 @@ Vertical Grading Feature Installation 2. Apply decorator 'enable_vertical_grading' to the next classes/functions - * lms.djangoapps.grades.new.subsection_grade.py: SubsectionGrade - * lms.djangoapps.grades.new.subsection_grade.py: ZeroSubsectionGrade - * common.lib.xmodule.xmodule.graders.py: AssignmentFormatGrader + * lms.djangoapps.grades.new.subsection_grade.py: CourseGradeBase + * common.lib.xmodule.xmodule: CourseFields * common.lib.xmodule.xmodule.vertical_block.py: VerticalBlock + * common.lib.xmodule.xmodule.graders.py: AssignmentFormatGrader * cms.djangoapps.contentstore.views.item.py: create_xblock_info - + * cms.djangoapps.models.settings.py: CourseMetadata Example: :: @@ -71,6 +71,8 @@ Vertical Grading Feature Installation 7. (Optional) Update staticfiles +8. (Optional) Set variable VERTICAL_GRADING_DEFAULT at SETTINGS to True/False. Works for courses which don't have records at NpoedGradingFeatures model. Default is False. + Passing Grade Feature Installation ------------------------------------- diff --git a/npoed_grading_features/enable_passing_grade.py b/npoed_grading_features/enable_passing_grade.py index b8c4202..67d7796 100644 --- a/npoed_grading_features/enable_passing_grade.py +++ b/npoed_grading_features/enable_passing_grade.py @@ -4,7 +4,7 @@ from .models import NpoedGradingFeatures, CoursePassingGradeUserStatus -NOT_PASSED_MESSAGE_TEMPLATE = _("You must earn {threshold_percent}% (got {student_percent}%) for {category}.") +MESSAGE_TEMPLATE = _("You must earn {threshold_percent}% (got {student_percent}%) for {category}.") def build_course_grading_model(class_): @@ -62,7 +62,7 @@ def inner_passing_grades(course_grade): passing_grades = dict((x['type'], x.get('passing_grade',0)) for x in graders) return passing_grades - def inner_categories_not_passed_messages(course_grade): + def inner_categories_get_messages(course_grade): passing_grades = inner_passing_grades(course_grade) breakdown = course_grade.grader_result['section_breakdown'] @@ -71,24 +71,25 @@ def inner_categories_not_passed_messages(course_grade): all(x in passing_grades for x in results) if not keys_match: # Error handling - return True + return [] - messages = [] + status_text_pairs = [] for category in results.keys(): - if results[category] < passing_grades[category]: - student_percent = int(round(results[category]*100)) - threshold_percent = int(round(passing_grades[category]*100)) - messages.append(NOT_PASSED_MESSAGE_TEMPLATE.format( + student_percent = int(round(results[category]*100)) + threshold_percent = int(round(passing_grades[category]*100)) + if threshold_percent: + current_status = results[category] < passing_grades[category] + current_text = MESSAGE_TEMPLATE.format( category=category, student_percent=student_percent, threshold_percent=threshold_percent - )) - return messages + ) + status_text_pairs.append((current_status, current_text)) + return status_text_pairs def inner_switch_to_default(course_grade): course_id = course_grade.course_data.course.id - result = not NpoedGradingFeatures.is_passing_grade_enabled(course_id) - return result + return not NpoedGradingFeatures.is_passing_grade_enabled(course_id) def _compute_passed(self, grade_cutoffs, percent): if inner_switch_to_default(self): @@ -96,13 +97,13 @@ def _compute_passed(self, grade_cutoffs, percent): nonzero_cutoffs = [cutoff for cutoff in grade_cutoffs.values() if cutoff > 0] success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None percent_passed = success_cutoff and percent >= success_cutoff - category_not_passed_messages = inner_categories_not_passed_messages(self) + message_pairs = inner_categories_get_messages(self) CoursePassingGradeUserStatus.set_passing_grade_status( user=self.user, course_key=self.course_data.course.id, - fail_status_messages=category_not_passed_messages + status_messages=message_pairs ) - category_passed = len(category_not_passed_messages) == 0 + category_passed = not any([failed for failed, text in message_pairs]) return percent_passed and category_passed def summary(self): @@ -123,14 +124,11 @@ def summary(self): if is_averaged_result and is_not_passed: student_percent = int(round(results[category]*100)) threshold_percent = int(round(passing_grades[category]*100)) - message = NOT_PASSED_MESSAGE_TEMPLATE.format( + message = MESSAGE_TEMPLATE.format( category=category, student_percent=student_percent, threshold_percent=threshold_percent ) - #message = u'Not passed. You must earn {percent} percent for this category to pass.'.format( - # percent=100*passing_grades[category] - #) section['mark'] = {'detail': message} return summary @@ -177,7 +175,7 @@ def is_course_passed(course, grade_summary=None, student=None, request=None): course_key = course.id if not NpoedGradingFeatures.is_passing_grade_enabled(course_key): return func(course, grade_summary, student, request) - failed_pass_grading = [] + has_failed = False if grade_summary: breakdown = grade_summary['section_breakdown'] failed_pass_grading = [] @@ -185,11 +183,12 @@ def is_course_passed(course, grade_summary=None, student=None, request=None): is_averaged_result = section.get('prominent', False) if is_averaged_result and 'mark' in section: if section['mark'].get('detail', None): - failed_pass_grading.append(section['mark']['detail']) + has_failed = True elif student: - failed_pass_grading = CoursePassingGradeUserStatus.get_passing_grade_status(course_key, student) + message_pairs = CoursePassingGradeUserStatus.get_passing_grade_status(course_key, student) + has_failed = any([failed for failed, text in message_pairs]) - is_category_grade_passed = not failed_pass_grading + is_category_grade_passed = not has_failed return is_category_grade_passed and func(course, grade_summary, student, request) return is_course_passed @@ -205,20 +204,20 @@ def _credit_course_requirements(course_key, student): credit_requirements = func(course_key, student) if not NpoedGradingFeatures.is_passing_grade_enabled(course_key): return credit_requirements - failed_categories = CoursePassingGradeUserStatus.get_passing_grade_status(course_key, student) - if not failed_categories: + message_pairs = CoursePassingGradeUserStatus.get_passing_grade_status(course_key, student) + if not message_pairs: return credit_requirements passing_grade_requirements = [{ "namespace": "passing_grade", "name": "", - "display_name": x, + "display_name": text, "criteria": "", "reason": "", - "status": "", - "status_date": None, + "status": "failed" if failed else "satisfied", + "status_date": " ", "order": None, - } for x in failed_categories] + } for failed, text in message_pairs] if credit_requirements is None: credit_requirements = { @@ -239,15 +238,11 @@ def _credit_course_requirements(course_key, student): } -def enable_passing_grade(class_): - """ - This decorator should be applied to the edx - classes/functions that are mentioned as keys in 'replaced'. - """ - if not settings.FEATURES.get("ENABLE_PASSING_GRADE", False): - return class_ - name = class_.__name__ +def enable_passing_grade(obj): + if not settings.FEATURES.get("ENABLE_GRADING_FEATURES", False): + return obj + name = obj.__name__ if name in replaced: constructor = replaced.get(name) - return constructor(class_) - return class_ + return constructor(obj) + return obj diff --git a/npoed_grading_features/enable_vertical_grading.py b/npoed_grading_features/enable_vertical_grading.py index 555b1ec..684668f 100644 --- a/npoed_grading_features/enable_vertical_grading.py +++ b/npoed_grading_features/enable_vertical_grading.py @@ -1,93 +1,59 @@ from collections import OrderedDict from functools import wraps -from lazy import lazy from django.conf import settings -from xblock.fields import Integer, Scope +from xblock.fields import Integer, Scope, Boolean -from .utils import get_vertical_score, vertical_grading_enabled, drop_minimal_vertical_from_subsection_grades +from .utils import find_drop_index, vertical_grading_enabled _ = lambda text: text -def build_subsection_grade(class_): - - def _vertical_compute_block_score( - self, - block_key, - course_structure, - submissions_scores, - csm_scores, - persisted_block=None, - ): - vertical_pseudo_problem_score = get_vertical_score( - block_key, - course_structure, - submissions_scores, - csm_scores, - persisted_block +def uniqueify(iterable): + """Looks weird, but that is how it is done in lms.djangoapps.grades""" + return OrderedDict([(item, None) for item in iterable]).keys() + + +def build_course_grade(cls): + class CourseVerticalGradeBase(cls): + def _get_subsection_grades(self, course_structure, chapter_key): + """ + Returns a list of subsection or vertical grades for the given chapter. + Checks course field to decide which grading model to apply + """ + vertical_mode = getattr(self.course_data.course, "vertical_grading", False) + grades = [] + for subsection_key in uniqueify(course_structure.get_children(chapter_key)): + if not vertical_mode: + grades.append(self._get_subsection_grade(course_structure[subsection_key])) + else: + subsection = course_structure[subsection_key] + vertical_keys = course_structure.get_children(subsection_key) + for vkey in vertical_keys: + vertical = course_structure[vkey] + grade = self._get_subsection_grade(vertical) + grade.format = subsection.format + grade.weight = vertical.weight + grades.append(grade) + return grades + return CourseVerticalGradeBase + + +def build_course_fields(cls): + default = getattr(settings, "VERTICAL_GRADING_DEFAULT", False) + # TODO: Later it can be replaced by waffle flags + + class NpoedCourseFields(cls): + vertical_grading = Boolean( + display_name=_("Vertical Grading"), + help=_("This field is not intended to be changed from AdvancedSettings"), + default=default, + scope=Scope.settings ) - if vertical_pseudo_problem_score: - self.problem_scores[block_key] = vertical_pseudo_problem_score + return NpoedCourseFields - def _compute_block_score(self, *args, **kwargs): - if vertical_grading_enabled(self.location.course_key): - return self._vertical_compute_block_score(*args, **kwargs) - else: - return self._problem_compute_block_score(*args, **kwargs) - class_._problem_compute_block_score = class_._compute_block_score - class_._vertical_compute_block_score = _vertical_compute_block_score - class_._compute_block_score = _compute_block_score - return class_ - - -def build_zero_subsection_grade(class_): - - def _vertical_problem_scores(self): - """ - Overrides the problem_scores member variable in order - to return empty scores for all scorable problems in the - course. - """ - from lms.djangoapps.grades.scores import possibly_scored #placed here to avoid circular import - - locations = OrderedDict() # dict of problem locations to ProblemScore - for block_key in self.course_data.structure.post_order_traversal( - filter_func=possibly_scored, - start_node=self.location, - ): - vertical_score = get_vertical_score( - block_key, - course_structure=self.course_data.structure, - submissions_scores={}, - csm_scores={}, - persisted_block=None, - ) - if vertical_score: - locations[block_key] = vertical_score - return locations - - def problem_scores(self): - if vertical_grading_enabled(self.location.course_key): - return self._vertical_problem_scores - else: - return self._old_problem_scores - - class_._old_problem_scores= class_.problem_scores - class_._vertical_problem_scores = lazy(_vertical_problem_scores) - class_.problem_scores = property(problem_scores) - return class_ - - -def build_vertical_block(class_): - class_.weight = Integer( - display_name=_("Weight"), - help=_( - "Defines the contribution of the vertical to the category score."), - default=0.0, - scope=Scope.settings - ) +def build_vertical_block(cls): def student_view(self, context): """ @@ -101,24 +67,31 @@ def student_view(self, context): if getattr(self,'weight', None): context['weight_string'] = _("Unit weight: {}").format(self.weight) return self._student_view(context) - class_._student_view = class_.student_view - class_.student_view = student_view - return class_ + cls._student_view = cls.student_view + cls.student_view = student_view + cls.weight = Integer( + display_name=_("Weight"), + help=_( + "Defines the contribution of the vertical to the category score."), + default=0.0, + scope=Scope.settings + ) + + return cls def build_create_xblock_info(func): """ This is decorator for cms.djangoapps.contentstore.item.py:create_xblock_info - It adds vertical block weight to the available for rendering info + It makes vertical block weight available for rendering info """ - @wraps(func) def wrapped(*args, **kwargs): xblock = kwargs.get('xblock', False) or args[0] xblock_info = func(*args, **kwargs) if not vertical_grading_enabled(xblock.location.course_key): return xblock_info - if xblock_info.get("category", False) == 'vertical': + if xblock_info.get("category", "") == 'vertical': weight = getattr(xblock, 'weight', 0) xblock_info['weight'] = weight xblock_info['vertical_grading'] = True @@ -132,39 +105,85 @@ def wrapped(*args, **kwargs): return wrapped -def build_assignment_format_grader(class_): - class_.problem_grade = class_.grade +def build_course_metadata(cls): + new_filtered_list = cls.FILTERED_LIST + new_filtered_list.append("vertical_grading") + + class NpoedCourseMetadata(cls): + """ + Hides vertical_grading attr from Advanced Settings at cms + """ + FILTERED_LIST = new_filtered_list - def get_course_id_from_grade_sheet(grade_sheet): - for category_dict in grade_sheet.values(): - for key in category_dict: - return key.course_key - return None + return cls - def grade(self, grade_sheet, generate_random_scores=False): - course_key = get_course_id_from_grade_sheet(grade_sheet) - if not vertical_grading_enabled(course_key): - return self.problem_grade(grade_sheet, generate_random_scores) - drop_count = self.drop_count - self.drop_count = 0 - subsection_grades = grade_sheet.get(self.type, {}) - for n in range(drop_count): - subsection_grades = drop_minimal_vertical_from_subsection_grades(subsection_grades) - grade_sheet[self.type] = subsection_grades - result = self.problem_grade(grade_sheet, generate_random_scores) - self.drop_count = drop_count - return result +def build_assignment_format_grader(cls): - class_.grade = grade - return class_ + class FlexibleNpoedGrader(cls): + """ + Flexible grader to apply vertical or sequential grading. + Decision is based on weight attr existence, which can only + be added at CourseVerticalGradeBase + """ + def grade(self, grade_sheet, generate_random_scores=False): + scores = grade_sheet.get(self.type, {}).values() + vertical_mode = all([hasattr(x,'weight') for x in scores]) + if not vertical_mode: + return super(FlexibleNpoedGrader, self).grade(grade_sheet, generate_random_scores) + + drop_count = self.drop_count + self.drop_count = 0 + result = super(FlexibleNpoedGrader, self).grade(grade_sheet, generate_random_scores) + if not scores: + return result + self.drop_count = drop_count + + breakdown = result['section_breakdown'] + + if len(breakdown) == 1: + # In this case AssignmentGrader returns only total score, we don't have grades per item + if self.drop_count == 0: + return result + else: + # AssignmentGrader didn't drop grade because we have turned off drop_count, we should do it manually + breakdown[0]['percent'] = 0 + breakdown[0]['detail'] = u"{section_type} = {percent:.0%}".format( + percent=0, + section_type=self.type, + ) + return { + 'section_breakdown': breakdown, + 'percent':0 + } + + percent = [x['percent'] for x in breakdown if 'prominent' not in x] + weights = [x.weight for x in scores] + for k in range(self.drop_count): + index = find_drop_index(percent, weights) + percent.pop(index) + weights.pop(index) + breakdown.pop(index) + total_weight = sum(weights) + if total_weight: + total_percent = sum([weights[i]*percent[i] for i in range(len(weights))])/total_weight + else: + total_percent = 0 + grading = { + "section_breakdown": breakdown, + "percent": total_percent + } + return grading + + return FlexibleNpoedGrader replaced = { - "SubsectionGrade": build_subsection_grade, - "ZeroSubsectionGrade": build_zero_subsection_grade, + "CourseGradeBase": build_course_grade, + "CourseFields": build_course_fields, "AssignmentFormatGrader": build_assignment_format_grader, "VerticalBlock": build_vertical_block, + "CourseMetadata": build_course_metadata, "create_xblock_info": build_create_xblock_info } diff --git a/npoed_grading_features/migrations/0002_auto_20180514_1013.py b/npoed_grading_features/migrations/0002_auto_20180514_1013.py new file mode 100644 index 0000000..f0ccb91 --- /dev/null +++ b/npoed_grading_features/migrations/0002_auto_20180514_1013.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('npoed_grading_features', '0001_initial'), + ] + + operations = [ + migrations.RemoveField( + model_name='coursepassinggradeuserstatus', + name='fail_status_messages', + ), + migrations.AddField( + model_name='coursepassinggradeuserstatus', + name='status_messages', + field=jsonfield.fields.JSONField(default={}, verbose_name=b'Message that specifies what user has to do to pass'), + ), + ] diff --git a/npoed_grading_features/models.py b/npoed_grading_features/models.py index c01e683..41b78f6 100644 --- a/npoed_grading_features/models.py +++ b/npoed_grading_features/models.py @@ -1,8 +1,11 @@ import json +import logging from django.contrib.auth.models import User from django.core.cache import cache from django.db import models +from jsonfield.fields import JSONField from opaque_keys.edx.keys import CourseKey +from xmodule.modulestore.django import modulestore class NpoedGradingFeatures(models.Model): @@ -53,6 +56,24 @@ def disable_passing_grade(cls, course_id): def disable_problem_best_score_grade(cls, course_id): cls._switch_feature(course_id, "problem_best_score", False) + @classmethod + def _push_vertical_grading_to_modulestore(cls, course_key, value): + course_key = CourseKey.from_string(cls._get_id(course_key)) + value = bool(value) + store = modulestore() + try: + course = store.get_course(course_key) + course.vertical_grading = value + store.update_item(course, 0) + except Exception as e: + message = "Failed to push vertical grading value '{}' for course '{}'.".format( + str(value), + str(course_key) + ) + message += "Are NpoedGradingFeatures enabled?" + message += "Exception:{}".format(str(e)) + logging.error(message) + @classmethod def get(cls, course_id, allow_cached=False): cid = cls._get_id(course_id) @@ -114,6 +135,7 @@ def _get_cache(cls, course_id): def save(self, *args, **kwargs): super(NpoedGradingFeatures, self).save(*args, **kwargs) + self._push_vertical_grading_to_modulestore(self.course_id, self.vertical_grading) self._set_cache() def __str__(self): @@ -129,7 +151,8 @@ class CoursePassingGradeUserStatus(models.Model): """ course_id = models.CharField(max_length=255) user = models.ForeignKey(User) - fail_status_messages = models.TextField( + status_messages = JSONField( + default={}, verbose_name="Message that specifies what user has to do to pass" ) @@ -145,18 +168,18 @@ def get_passing_grade_status(cls, course_key, user): )) try: row = cls.objects.get(course_id=course_id, user=user) - messages = json.loads(row.fail_status_messages) + messages = row.status_messages except cls.DoesNotExist: messages = tuple("You progress is not processed yet") return messages @classmethod - def set_passing_grade_status(cls, course_key, user, fail_status_messages): + def set_passing_grade_status(cls, course_key, user, status_messages): course_id = str(course_key) if not NpoedGradingFeatures.is_passing_grade_enabled(course_id): raise ValueError("Passing grade is not enabled for course {}.".format( course_id )) row, created = cls.objects.get_or_create(course_id=course_id, user=user) - row.fail_status_messages = json.dumps(fail_status_messages) + row.status_messages = status_messages row.save() diff --git a/npoed_grading_features/tests/test_utils.py b/npoed_grading_features/tests/test_utils.py index a4c3a12..1e84d69 100644 --- a/npoed_grading_features/tests/test_utils.py +++ b/npoed_grading_features/tests/test_utils.py @@ -1,11 +1,17 @@ +from collections import namedtuple +from openedx.core.djangolib.testing.utils import get_mock_request +from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.partitions.partitions import ( - Group, UserPartition, MINIMUM_STATIC_PARTITION_ID + UserPartition, MINIMUM_STATIC_PARTITION_ID ) from lms.djangoapps.grades.tests.utils import answer_problem from ..models import NpoedGradingFeatures -from ..utils import _find_worst_score +from ..utils import find_drop_index + + +TestGrade = namedtuple('TestGrade', ["earn", "max", "weight"]) class ContentGroupsMixin(object): @@ -173,44 +179,33 @@ def get_unit_problem_pairs(unit_tree): return all_earned, all_max def grade_subsection(subsection_tree): - element_earned, element_max = [], [] - + grades = [] for unit_name, unit_pair in subsection_tree.iteritems(): weight, unit_tree = unit_pair current_earned, current_max = get_unit_problem_pairs(unit_tree) - if not current_max: - continue - if enable_vertical: - if not weight: - continue - current_earned = [sum(current_earned)*weight/sum(current_max)] - current_max = [weight] - element_earned.extend(current_earned) - element_max.extend(current_max) - if not element_max: - return None - return zip(element_earned, element_max) - - def compute_subsection_percent(pairs): - if pairs: - return sum(x[0] for x in pairs) / sum(x[1] for x in pairs) - - def drop_minimal_vertical_from_subsection_grades_mimic(tuple_of_subsection_unit_score_pairs): - """ - :param tuple_of_subsection_unit_score_pairs_lists: - ( - [(1,3), (0,1), (None, None)], - [(1,1, (1,10)] - ) - """ - _tree = {} - for subsection_num, subsection_list in enumerate(tuple_of_subsection_unit_score_pairs): - subsection_dict = dict((num, grade) for num, grade in enumerate(subsection_list)) - _tree[subsection_num] = subsection_dict + if not enable_vertical: + weight = 1 + if weight and current_max: + grades.append(TestGrade(sum(current_earned), sum(current_max), weight)) - best_drop_index = _find_worst_score(_tree) - if best_drop_index is not None: - tuple_of_subsection_unit_score_pairs[best_drop_index[0]].pop(best_drop_index[1]) + if enable_vertical: + return grades + subsection_grade = TestGrade(sum([x.earn for x in grades]), sum([x.max for x in grades]), 1) + return [subsection_grade] + + def drop_minimal(grades_list): + pc = [x.earn/x.max for x in grades_list] + w = [x.weight for x in grades_list] + ind = find_drop_index(pc, w) + grades_list.pop(ind) + + def weighted_score(grades_list): + percent = lambda x: x.earn/x.max if x.max else 0 + top = sum([percent(g)*g.weight for g in grades_list]) + bottom = sum([g.weight for g in grades_list]) + if not bottom: + return 0 + return top/bottom score_by_assignment_category = {} for _, section_tree in tree.iteritems(): @@ -220,35 +215,29 @@ def drop_minimal_vertical_from_subsection_grades_mimic(tuple_of_subsection_unit_ continue if assignment_category not in score_by_assignment_category: score_by_assignment_category[assignment_category] = [] - subsection_score = grade_subsection(subsection_tree) - if subsection_score is not None: - score_by_assignment_category[assignment_category].append(subsection_score) + grades = grade_subsection(subsection_tree) + score_by_assignment_category[assignment_category].extend(grades) + course_score = 0 assignment_category_meta = dict( ((x["type"], {"weight": x["weight"], "drop_count": x["drop_count"]}) - for x in self.course.grading_policy["GRADER"]) + for x in self.course.grading_policy["GRADER"]) ) for assignment_category in score_by_assignment_category: - if enable_vertical: - for _ in range(int(assignment_category_meta[assignment_category]["drop_count"])): - - drop_minimal_vertical_from_subsection_grades_mimic(score_by_assignment_category[assignment_category]) - subsection_percents = [compute_subsection_percent(x) for x in score_by_assignment_category[assignment_category]] - subsection_percents = sorted(subsection_percents, key=lambda x: -1 if x is None else x) - if not enable_vertical: - start_id = assignment_category_meta[assignment_category]["drop_count"] - subsection_percents = subsection_percents[start_id::] - - if subsection_percents: - weight = assignment_category_meta[assignment_category]["weight"] - subsection_percents = [ x for x in subsection_percents if x is not None] - average_score = sum(subsection_percents)/len(subsection_percents) if subsection_percents else 0 - assignment_score = weight * average_score - course_score += assignment_score + drop_count = int(assignment_category_meta[assignment_category]["drop_count"]) + weight = assignment_category_meta[assignment_category]["weight"] + + for _ in range(drop_count): + drop_minimal(score_by_assignment_category[assignment_category]) + assignment_score = weighted_score(score_by_assignment_category[assignment_category]) + course_score += assignment_score * weight course_score = round(course_score * 100 + 0.05) / 100 return course_score def _enable_if_needed(self, enable_vertical): if enable_vertical: - NpoedGradingFeatures.enable_vertical_grading(str(self.course.id)) \ No newline at end of file + NpoedGradingFeatures.enable_vertical_grading(str(self.course.id)) + # needed in tests only + self.course.vertical_grading = True + #self.store.update_item(self.course, 0) diff --git a/npoed_grading_features/tests/test_vertical_grading.py b/npoed_grading_features/tests/test_vertical_grading.py index 78df00f..4cb093b 100644 --- a/npoed_grading_features/tests/test_vertical_grading.py +++ b/npoed_grading_features/tests/test_vertical_grading.py @@ -1,6 +1,6 @@ import ddt from mock import patch - +from django.conf import settings from openedx.core.djangolib.testing.utils import get_mock_request from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory, CourseData @@ -14,6 +14,7 @@ from .test_utils import BuildCourseMixin, ContentGroupsMixin +@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) class TestExpectedGrading(ModuleStoreTestCase, BuildCourseMixin): """ Check that our testing method in BuildCourseMixin correctly calculates grade @@ -108,7 +109,7 @@ def test_percents_coincide(self): model_calculated_pc = self._grade_tree(tree, enable_vertical=False) self.assertEqual(pc, model_calculated_pc) - +@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.ddt class TestCourseBuilding(ModuleStoreTestCase, BuildCourseMixin): """ @@ -138,15 +139,16 @@ def test_percent(self, enable_vertical): "b": ("Homework", { "d": (0., {"h": (2., 5.), "i": (3., 5.),}), "e": (1., {"j": (0., 1.), "k": (None, None), "l":(1.,3)}), - "f": (0., {"m":(None, None)}) + "f": (0., {"m": (None, None)}) }), "c": ("Homework", { "g": (0, {"n": (6., 10)}) }) } } - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) + self._check_tree(tree, self.course) course_grade = CourseGradeFactory().create(self.request.user, self.course) pc = course_grade.percent @@ -164,6 +166,7 @@ def test_percent(self, enable_vertical): self.assertEqual(pc, model_calculated_pc) +@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.ddt class TestVerticalGrading(ModuleStoreTestCase, BuildCourseMixin, ContentGroupsMixin): """ @@ -190,8 +193,8 @@ def test_simple(self, enable_vertical): } } self._update_grading_policy() - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) @@ -211,8 +214,8 @@ def test_unanswered(self, enable_vertical): } } self._update_grading_policy() - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) @@ -228,13 +231,13 @@ def test_no_graded_subsections(self, enable_vertical): tree = { "a": { "b": (None, { - "c": (0., {"d": (0, 1.), "e": (0, 1.)}), + "c": (1., {"d": (0, 1.), "e": (0, 1.)}), }), } } self._update_grading_policy() - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) @@ -257,10 +260,13 @@ def test_has_zero_weights(self, enable_vertical): } } self._update_grading_policy() - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) + # TODO: somehow .create doesn't work. Why? + # pc = CourseGradeFactory().create(self.request.user, self.course).percent pc = CourseGradeFactory().create(self.request.user, self.course).percent + model_calculated_pc = self._grade_tree(tree, enable_vertical) self.assertEqual(pc, model_calculated_pc) expected_pc = 0.8 if enable_vertical else 0.5 @@ -287,8 +293,8 @@ def test_several_subsections(self, enable_vertical): } } self._update_grading_policy() - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent @@ -337,20 +343,23 @@ def test_several_assignment_categories(self, enable_vertical): "Pass": 0.5, }, } + self._build_from_tree(tree) self._update_grading_policy(grading_policy) self._enable_if_needed(enable_vertical) - self._build_from_tree(tree) self._check_tree(tree, self.course) + # TODO: somehow .create doesn't work. Why? + # pc = CourseGradeFactory().create(self.request.user, self.course).percent pc = CourseGradeFactory().create(self.request.user, self.course).percent - model_calculated_pc = self._grade_tree(tree, enable_vertical) - self.assertEqual(pc, model_calculated_pc) expected_pc_vertical = 0.4*((1.*.5 + 0.*.5)/1) + 0.6*((1.*1 + .5*3)/4) expected_pc_non_vertical = 0.4*.5 + 0.6*0.75 expected_pc = expected_pc_vertical if enable_vertical else expected_pc_non_vertical self.assertEqual(pc, round(expected_pc + 0.05/100, 2)) # edx grade rounding, fails otherwise + model_calculated_pc = self._grade_tree(tree, enable_vertical) + self.assertEqual(pc, model_calculated_pc) + @ddt.data(True, False) def test_droppable_subsections(self, enable_vertical): """ @@ -358,17 +367,17 @@ def test_droppable_subsections(self, enable_vertical): """ tree = { "a": { - "b1": ("Homework", { # NW 0.5; W: 0.8 - "c1": (1., {"d1": (0., 1.), "e1": (0., 1.)}), # W:0/1 - "f1": (4., {"g1": (1., 1.), "h1": (1., 1.)}), # W:4/4 + "b1": ("Homework", { # NW 0.5; + "c1": (1., {"d1": (0., 1.), "e1": (0., 1.)}), # W:0. + "f1": (4., {"g1": (1., 1.), "h1": (1., 1.)}), # W:1. }), - "b2": ("Homework", { # NW 0.75: W: 1 - "c2": (1., {"d2": (1., 1.), "e2": (1., 1.)}), # W:1/1 - "f2": (4., {"g2": (1., 1.), "h2": (0., 1.)}), # W:2/4 - + "b2": ("Homework", { # NW 0.75 + "c2": (1., {"d2": (1., 1.), "e2": (1., 1.)}), # W:1. + "f2": (4., {"g2": (1., 1.), "h2": (0., 1.)}), # W:0.5 }), - "b3": ("Homework", { # NW 0.25; W: 0.5 - "c3": (1., {"d3": (0., 1.), "e3": (1., 1.)}), # W:0.5/1 - "f3": (4., {"g3": (0., 1.), "h3": (0., 1.)}), # W:0./4 - + "b3": ("Homework", { # NW 0.25; + "c3": (1., {"d3": (0., 1.), "e3": (1., 1.)}), # W:0.5 + "f3": (4., {"g3": (0., 1.), "h3": (0., 1.)}), # W:0. }), } } @@ -388,20 +397,26 @@ def test_droppable_subsections(self, enable_vertical): }, } + self._build_from_tree(tree) self._update_grading_policy(grading_policy) self._enable_if_needed(enable_vertical) - self._build_from_tree(tree) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent - model_calculated_pc = self._grade_tree(tree, enable_vertical) - self.assertEqual(pc, model_calculated_pc) - expected_pc_vertical = (1. + 0.8 + 0.5)/3. + #checked all drop index - the best is [1, 1, 1, 0, 1, 0] + drop = [1, 1, 1, 0, 1, 0] + weight = [1, 4, 1, 4, 1, 4] + gpc = [0, 1, 1, 0.5, 0.5, 0] + expected_pc_vertical = sum([drop[n]*weight[n]*gpc[n] for n in range(len(gpc))]) + expected_pc_vertical /=sum([drop[n]*weight[n] for n in range(len(gpc))]) expected_pc_non_vertical = 0.75 expected_pc = expected_pc_vertical if enable_vertical else expected_pc_non_vertical self.assertEqual(pc, round(expected_pc + 0.05 / 100, 2)) + model_calculated_pc = self._grade_tree(tree, enable_vertical) + self.assertEqual(pc, model_calculated_pc) + @ddt.data(True, False) def test_drop_last_element(self, enable_vertical): """ @@ -431,15 +446,14 @@ def test_drop_last_element(self, enable_vertical): }, } + self._build_from_tree(tree) self._update_grading_policy(grading_policy) self._enable_if_needed(enable_vertical) - self._build_from_tree(tree) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) self.assertEqual(pc, model_calculated_pc) - self.assertEqual(pc, 0) @ddt.data(True, False) @@ -475,9 +489,9 @@ def test_drop_last_element_with_several_ss(self, enable_vertical): }, } + self._build_from_tree(tree) self._update_grading_policy(grading_policy) self._enable_if_needed(enable_vertical) - self._build_from_tree(tree) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent @@ -520,14 +534,16 @@ def test_drop_optimal(self, enable_vertical): "Pass": 0.5, }, } + self._build_from_tree(tree) self._update_grading_policy(grading_policy) self._enable_if_needed(enable_vertical) - self._build_from_tree(tree) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) self.assertEqual(pc, model_calculated_pc) - expected_pc = (1. + 0.5)/2 if enable_vertical else 1 + #drop with weight 4 + grade_round = lambda x: round(x + 0.05 / 100, 2) + expected_pc = grade_round ((1.*1 + 1*1 + 0*1)/(1+1+1)) if enable_vertical else 1 self.assertEqual(pc, expected_pc) @ddt.data(True, False) @@ -559,11 +575,11 @@ def test_drop_over_min_count(self, enable_vertical): }, } self._update_grading_policy(grading_policy) - self._enable_if_needed(enable_vertical) self._build_from_tree(tree) + self._enable_if_needed(enable_vertical) self._check_tree(tree, self.course) pc = CourseGradeFactory().create(self.request.user, self.course).percent model_calculated_pc = self._grade_tree(tree, enable_vertical) self.assertEqual(pc, model_calculated_pc) expected_pc = 0. - self.assertEqual(pc, expected_pc) + self.assertEqual(pc, expected_pc) \ No newline at end of file diff --git a/npoed_grading_features/utils.py b/npoed_grading_features/utils.py index 077872b..c5ecf15 100644 --- a/npoed_grading_features/utils.py +++ b/npoed_grading_features/utils.py @@ -10,168 +10,22 @@ def vertical_grading_enabled(course_id): VERTICAL_CATEGORY = 'vertical' -def get_vertical_score( - block_key, - course_structure, - submissions_scores, - csm_scores, - persisted_block=None -): - """ - In vertical grading we the basic scoring element is Unit(vertical) instead of problem. - To implement this we emulate vertical scoring by single ProblemScore: - if grading is called for vertical, we take all it's descendant problems, and set - unit.raw_earned = sum(problem.raw_earned), unit.raw_possible = sum(problem.raw_possible). - Resulted fake ProblemScore that represent unit scores are graded as usually to calculate - subsection grades. - """ - from lms.djangoapps.grades.scores import get_score, ProblemScore # placed here to avoid circular import - - if block_key.category != VERTICAL_CATEGORY: - return - vertical_weight = getattr(course_structure[block_key], "weight", None) - if not vertical_weight: - return - children_keys = course_structure.get_children(block_key) - children_scores = [] - for child_key in children_keys: - try: - block = course_structure[child_key] - except KeyError: - # It's possible that the user's access to that - # block has changed since the subsection grade - # was last persisted. - pass - else: - if getattr(block, 'has_score', False): - problem_score = get_score( - submissions_scores, - csm_scores, - persisted_block, - block, - ) - if problem_score: - children_scores.append(problem_score) - if not children_scores: - return - vertical_possible = sum(score.possible for score in children_scores) - vertical_earned = sum(score.earned for score in children_scores) - weighted_earned = vertical_weight * float(vertical_earned) / vertical_possible - weighted_possible = vertical_weight - inner_first_attempted = list(score.first_attempted for score in children_scores) - vertical_attempted = max(inner_first_attempted) if inner_first_attempted else None - vertical_graded = any(score.graded for score in children_scores) and bool(weighted_possible) - vertical_pseudo_problem = ProblemScore( - raw_earned=weighted_earned, - raw_possible=weighted_possible, - weighted_earned=weighted_earned, - weighted_possible=weighted_possible, - weight=1, - graded=vertical_graded, - first_attempted=vertical_attempted - ) - return vertical_pseudo_problem - - -def drop_minimal_vertical_from_subsection_grades(subsection_grades): - """ - This function finds the worst block and drops it from the subsection grades. - The definition of the worst block itself is moved into _find_worst_score - which takes not a SubsectionGrades but pairs to ease testing. - """ - tree = _build_tree_from_grades(subsection_grades) - best_score_drop_index = _find_worst_score(tree) - if best_score_drop_index is None: - return subsection_grades - modified_grade = subsection_grades[best_score_drop_index[0]] - subtracted_score = modified_grade.problem_scores.pop(best_score_drop_index[1]) - modified_grade.graded_total.earned -= subtracted_score.earned - modified_grade.graded_total.possible -= subtracted_score.possible - - modified_grade.all_total.earned -= subtracted_score.earned - modified_grade.all_total.possible -= subtracted_score.possible - if not modified_grade.graded_total.possible: - subsection_grades.pop(best_score_drop_index[0]) - return subsection_grades - - -def _find_worst_score(subsection_grades_tree): - """ - Takes structure of category grading tree in - vertical grading case: - { - SubsectionKey1: {BlockKey1:(earned, possible), BlockKey2: ...}, - SubsectionKey2: {...} - } - returns: - N, BlockKey - subsection number, unit's block key - None - if no need to drop anything(no grading elements) - - """ - best_score_drop_index = None - subsection_scores = {} - for subsection_key, subsection_unit_grades in subsection_grades_tree.iteritems(): - earned, possible = zip(*subsection_unit_grades.values()) - subsection_scores[subsection_key] = (sum(earned), sum(possible)) - - def grade_without(subsection_key, unit_key): - removed_grade = subsection_grades_tree[subsection_key][unit_key] - changed_score = subsection_scores[subsection_key] - modified_subsection_score = ( - changed_score[0] - removed_grade[0], - changed_score[1] - removed_grade[1] - ) - if modified_subsection_score[1]: # there are at least two units in subsection - rest_percents = [ - (x[0] / x[1]) if (key != subsection_key) - else (modified_subsection_score[0]/modified_subsection_score[1]) - for key, x in subsection_scores.iteritems() - ] - else: # this is the last unit - rest_percents = [ - (x[0] / x[1]) - for key, x in subsection_scores.iteritems() - if (key != subsection_key) - ] - - if not rest_percents: - # this is the only possible drop, give him the highest rank - return 1. - else: - return sum(rest_percents)/len(rest_percents) - - best_score = sum([x[0] for x in subsection_scores.values()]) / sum([x[1] for x in subsection_scores.values()]) - - for subsection_key, subsection_unit_grades in subsection_grades_tree.iteritems(): - for unit_key in subsection_unit_grades: - score = grade_without(subsection_key, unit_key) - if score >= best_score: - best_score = score - best_score_drop_index = (subsection_key, unit_key) - return best_score_drop_index - - -def _build_tree_from_grades(subsection_grades): - """ - Parses subsection grades - :param subsection_grades: - { - SubsectionKey1: SubsectionGrade(), - SubsectionKey2: SubsectionGrade()... - } - :return: - { - SubsectionKey1: {UnitKey1: tuple(earned, possible), UnitKey2:...}, - ... - } - """ - tree = {} - for key, grade in subsection_grades.iteritems(): - subtree = {} - for block_key, problem_score in grade.problem_scores.items(): - subtree[block_key] = (problem_score.earned, problem_score.possible) - tree[key] = subtree - return tree +def find_drop_index(percents, weights): + """ + G = sum(w[i]p[i])/sum(w[i]) + G'[j] = sum(w[i]p[i])/sum(w[i]) : i!=j + gain[j] = G'[j] - G + return: max(delta) + """ + length = len(percents) + if len(percents) == len(weights) == 1: + return 0 + top = sum([percents[i]*weights[i] for i in range(length)]) + bot = sum(weights) + gain = [] + for pair in zip(weights, percents): + gain.append(pair[0] * (top - bot * pair[1]) / (bot - pair[0])) + return gain.index(max(gain)) def patch_function(func, implementation, dynamic_key=None):