From 940a337dd3f635788d0dada3611e789f0983dd89 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 29 Aug 2013 12:14:30 -0400 Subject: [PATCH 1/3] Properly display student progress --- .../combined_open_ended_modulev1.py | 52 +++++++++++++------ .../xmodule/tests/test_combined_open_ended.py | 24 +++++++++ 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 6ab8db8334..becb0fd170 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -823,6 +823,16 @@ class CombinedOpenEndedV1Module(): """ return (self.state == self.DONE or self.ready_to_reset) and self.is_scored + def get_weight(self): + """ + Return the weight of the problem. The old default weight was None, so set to 1 in that case. + Output - int weight + """ + weight = self.weight + if weight is None: + weight = 1 + return weight + def get_score(self): """ Score the student received on the problem, or None if there is no @@ -837,9 +847,7 @@ class CombinedOpenEndedV1Module(): score = None #The old default was None, so set to 1 if it is the old default weight - weight = self.weight - if weight is None: - weight = 1 + weight = self.get_weight() if self.is_scored: # Finds the maximum score of all student attempts and keeps it. score_mat = [] @@ -879,27 +887,41 @@ class CombinedOpenEndedV1Module(): return score_dict def max_score(self): - ''' Maximum score. Two notes: - - * This is generic; in abstract, a problem could be 3/5 points on one - randomization, and 5/7 on another - ''' + """ + Maximum score possible in this module. Returns the max score if finished, None if not. + """ max_score = None if self.check_if_done_and_scored(): max_score = self._max_score return max_score def get_progress(self): - ''' Return a progress.Progress object that represents how far the + """ + Generate a progress object. Progress objects represent how far the student has gone in this module. Must be implemented to get correct - progress tracking behavior in nesting modules like sequence and - vertical. + progress tracking behavior in nested modules like sequence and + vertical. This behavior is consistent with capa. - If this module has no notion of progress, return None. - ''' - progress_object = Progress(self.current_task_number, len(self.task_xml)) + If the module is unscored, return None (consistent with capa). + """ - return progress_object + d = self.get_score() + score = d['score'] + total = d['total'] + + if total > 0 and self.is_scored: + if self.weight is not None: + # scale score and total by weight/total: + score = score * self.get_weight() / total + total = self.get_weight() + + try: + return Progress(score, total) + except (TypeError, ValueError): + log.exception("Got bad progress") + return None + + return None def out_of_sync_error(self, data, msg=''): """ diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 26c248f71d..d97d62d7d3 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -22,6 +22,7 @@ from xmodule.open_ended_grading_classes.grading_service_module import GradingSer from xmodule.combined_open_ended_module import CombinedOpenEndedModule from xmodule.modulestore import Location from xmodule.tests import get_test_system, test_util_open_ended +from xmodule.progress import Progress from xmodule.tests.test_util_open_ended import (MockQueryDict, DummyModulestore, TEST_STATE_SA_IN, MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID, TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE) @@ -480,6 +481,29 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): max_score = self.combinedoe_container.max_score() self.assertEqual(max_score, None) + def test_container_get_progress(self): + """ + See if we can get the progress from the actual xmodule + """ + progress = self.combinedoe_container.max_score() + self.assertEqual(progress, None) + + def test_get_progress(self): + """ + Test if we can get the correct progress from the combined open ended class + """ + self.combinedoe.update_task_states() + self.combinedoe.state = "done" + self.combinedoe.is_scored = True + progress = self.combinedoe.get_progress() + self.assertIsInstance(progress, Progress) + + #progress._a is the score of the xmodule, which is 0 right now + self.assertEqual(progress._a, 0) + + #progress._b is the max_score (which is 1), divided by the weight (which is 1) + self.assertEqual(progress._b, 1) + def test_container_weight(self): """ Check the problem weight in the container From a06226aad65713004b7d751fa6d6d7d95e9c7b71 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 29 Aug 2013 16:34:17 -0400 Subject: [PATCH 2/3] Address review comments --- .../combined_open_ended_modulev1.py | 10 ++-------- .../xmodule/xmodule/tests/test_combined_open_ended.py | 4 ++-- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index becb0fd170..63c58135dc 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -906,17 +906,11 @@ class CombinedOpenEndedV1Module(): """ d = self.get_score() - score = d['score'] - total = d['total'] - if total > 0 and self.is_scored: - if self.weight is not None: - # scale score and total by weight/total: - score = score * self.get_weight() / total - total = self.get_weight() + if d['total'] > 0 and self.is_scored: try: - return Progress(score, total) + return Progress(d['score'], d['total']) except (TypeError, ValueError): log.exception("Got bad progress") return None diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index d97d62d7d3..8a32f7e822 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -498,10 +498,10 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): progress = self.combinedoe.get_progress() self.assertIsInstance(progress, Progress) - #progress._a is the score of the xmodule, which is 0 right now + # progress._a is the score of the xmodule, which is 0 right now. self.assertEqual(progress._a, 0) - #progress._b is the max_score (which is 1), divided by the weight (which is 1) + # progress._b is the max_score (which is 1), divided by the weight (which is 1). self.assertEqual(progress._b, 1) def test_container_weight(self): From 9b57ef0b5451e92ef8237bdafa54fb1b507321c4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 29 Aug 2013 17:28:14 -0400 Subject: [PATCH 3/3] Pep8 fixes --- .../combined_open_ended_modulev1.py | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 63c58135dc..340012a042 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -5,11 +5,11 @@ from xmodule.timeinfo import TimeInfo from xmodule.capa_module import ComplexEncoder from xmodule.progress import Progress from xmodule.stringify import stringify_children -import self_assessment_module -import open_ended_module +from xmodule.open_ended_grading_classes import self_assessment_module +from xmodule.open_ended_grading_classes import open_ended_module from functools import partial from .combined_open_ended_rubric import CombinedOpenEndedRubric, GRADER_TYPE_IMAGE_DICT, HUMAN_GRADER_TYPE, LEGEND_LIST -from peer_grading_service import PeerGradingService, MockPeerGradingService, GradingServiceError +from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, MockPeerGradingService, GradingServiceError log = logging.getLogger("mitx.courseware") @@ -547,6 +547,11 @@ class CombinedOpenEndedV1Module(): return last_response_dict def extract_human_name_from_task(self, task_xml): + """ + Given the xml for a task, pull out the human name for it. + Input: xml string + Output: a human readable task name (ie Self Assessment) + """ tree = etree.fromstring(task_xml) payload = tree.xpath("/openended/openendedparam/grader_payload") if len(payload) == 0: @@ -644,7 +649,13 @@ class CombinedOpenEndedV1Module(): all_responses = [] success, can_see_rubric, error = self.check_if_student_has_done_needed_grading() if not can_see_rubric: - return {'html' : self.system.render_template('{0}/combined_open_ended_hidden_results.html'.format(self.TEMPLATE_DIR), {'error' : error}), 'success' : True, 'hide_reset' : True} + return { + 'html': self.system.render_template( + '{0}/combined_open_ended_hidden_results.html'.format(self.TEMPLATE_DIR), + {'error': error}), + 'success': True, + 'hide_reset': True + } contexts = [] rubric_number = self.current_task_number @@ -717,6 +728,9 @@ class CombinedOpenEndedV1Module(): return json.dumps(d, cls=ComplexEncoder) def get_current_state(self, data): + """ + Gets the current state of the module. + """ return self.get_context() def get_last_response_ajax(self, data): @@ -866,7 +880,6 @@ class CombinedOpenEndedV1Module(): if len(score_mat) > 0: # Currently, assume that the final step is the correct one, and that those are the final scores. # This will change in the future, which is why the machinery above exists to extract all scores on all steps - # TODO: better final score handling. scores = score_mat[-1] score = max(scores) else: