From 588b27c9dc6911e9b8e0728f274b155cf9674c46 Mon Sep 17 00:00:00 2001 From: Julian Arni Date: Mon, 11 Mar 2013 11:17:12 -0400 Subject: [PATCH 01/17] Added Descriptor method, fixed order issue. --- common/lib/xmodule/xmodule/foldit_module.py | 14 ++++++++++---- lms/djangoapps/foldit/models.py | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index 920a5aed6d..2bcdda5bc2 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -86,7 +86,10 @@ class FolditModule(XModule): """ from foldit.models import Score - return [(e['username'], e['score']) for e in Score.get_tops_n(10)] + leaders = [(e['username'], e['score']) for e in Score.get_tops_n(10)] + leaders.sort(key=lambda x: x[1]) + + return leaders def get_html(self): """ @@ -173,7 +176,10 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor): @classmethod def definition_from_xml(cls, xml_object, system): - """ - Get the xml_object's attributes. - """ + """ Get the xml_object's attributes. """ + return {'metadata': xml_object.attrib} + + def definition_to_xml(self): + xml_object = etree.Element('foldit') + return xml_object diff --git a/lms/djangoapps/foldit/models.py b/lms/djangoapps/foldit/models.py index 7041be1446..0dce956756 100644 --- a/lms/djangoapps/foldit/models.py +++ b/lms/djangoapps/foldit/models.py @@ -59,7 +59,7 @@ class Score(models.Model): scores = Score.objects \ .filter(puzzle_id__in=puzzles) \ .annotate(total_score=models.Sum('best_score')) \ - .order_by('-total_score')[:n] + .order_by('total_score')[:n] num = len(puzzles) return [{'username': s.user.username, From 6e92666a203ed9ec149a44458c24e6d3d1a6be97 Mon Sep 17 00:00:00 2001 From: Julian Arni Date: Mon, 11 Mar 2013 14:57:19 -0400 Subject: [PATCH 02/17] Fixed score order. Less is more. --- lms/djangoapps/foldit/tests.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/foldit/tests.py b/lms/djangoapps/foldit/tests.py index 7127651601..afdd678f06 100644 --- a/lms/djangoapps/foldit/tests.py +++ b/lms/djangoapps/foldit/tests.py @@ -143,11 +143,12 @@ class FolditTestCase(TestCase): def test_SetPlayerPuzzleScores_manyplayers(self): """ Check that when we send scores from multiple users, the correct order - of scores is displayed. + of scores is displayed. Note that, before being processed by + display_score, lower scores are better. """ puzzle_id = ['1'] - player1_score = 0.07 - player2_score = 0.08 + player1_score = 0.08 + player2_score = 0.02 response1 = self.make_puzzle_score_request(puzzle_id, player1_score, self.user) @@ -164,8 +165,12 @@ class FolditTestCase(TestCase): self.assertEqual(len(top_10), 2) # Top score should be player2_score. Second should be player1_score - self.assertEqual(top_10[0]['score'], Score.display_score(player2_score)) - self.assertEqual(top_10[1]['score'], Score.display_score(player1_score)) + self.assertAlmostEqual(top_10[0]['score'], + Score.display_score(player2_score), + delta=0.5) + self.assertAlmostEqual(top_10[1]['score'], + Score.display_score(player1_score), + delta=0.5) # Top score user should be self.user2.username self.assertEqual(top_10[0]['username'], self.user2.username) From cc2c26b9240271eb79e334b4c2768ab7684dc592 Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 11 Mar 2013 18:04:21 -0400 Subject: [PATCH 03/17] reverted change to definition to/from xml --- common/lib/xmodule/xmodule/foldit_module.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index 2bcdda5bc2..65632fa031 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -176,10 +176,9 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor): @classmethod def definition_from_xml(cls, xml_object, system): - """ Get the xml_object's attributes. """ + """ + Get the xml_object's attributes. + """ return {'metadata': xml_object.attrib} - def definition_to_xml(self): - xml_object = etree.Element('foldit') - return xml_object From 00c35c95a4140741da9669c7d01965cba98a7170 Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 11 Mar 2013 18:05:10 -0400 Subject: [PATCH 04/17] whitespace back to original --- common/lib/xmodule/xmodule/foldit_module.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index 65632fa031..152930876d 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -176,9 +176,7 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor): @classmethod def definition_from_xml(cls, xml_object, system): - """ + """ Get the xml_object's attributes. """ - return {'metadata': xml_object.attrib} - From ad92eb4d022da3c7aba6df37929905621b2d4610 Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 11 Mar 2013 18:05:34 -0400 Subject: [PATCH 05/17] whitespace back to original. for real though. --- common/lib/xmodule/xmodule/foldit_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index 152930876d..37255bd5cb 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -177,6 +177,6 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor): @classmethod def definition_from_xml(cls, xml_object, system): """ - Get the xml_object's attributes. + Get the xml_object's attributes. """ return {'metadata': xml_object.attrib} From b06615298fa71ce6e70d0b756ba1ab6530e8d82f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 11 Mar 2013 15:24:14 -0700 Subject: [PATCH 06/17] Add logging to debug strange cohort behavior on prod. Strange behavior: - There are 20 cohorts that users should be randomly assigned into in HeroesX - Almost all, but not all (e.g. ~3300 / 3400) users end up in the same group. - testing manually locally and on prod in a django shell shows nothing wrong --- common/djangoapps/course_groups/cohorts.py | 24 ++++++++----- .../djangoapps/course_groups/tests/tests.py | 35 +++++++++++++++++-- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index f0234ec71a..c362ed4e89 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -65,23 +65,23 @@ def is_commentable_cohorted(course_id, commentable_id): ans)) return ans - + def get_cohorted_commentables(course_id): """ Given a course_id return a list of strings representing cohorted commentables """ course = courses.get_course_by_id(course_id) - + if not course.is_cohorted: # this is the easy case :) ans = [] - else: + else: ans = course.cohorted_discussions return ans - - + + def get_cohort(user, course_id): """ Given a django User and a course_id, return the user's cohort in that @@ -120,7 +120,8 @@ def get_cohort(user, course_id): return None choices = course.auto_cohort_groups - if len(choices) == 0: + n = len(choices) + if n == 0: # Nowhere to put user log.warning("Course %s is auto-cohorted, but there are no" " auto_cohort_groups specified", @@ -128,12 +129,19 @@ def get_cohort(user, course_id): return None # Put user in a random group, creating it if needed - group_name = random.choice(choices) + choice = random.randrange(0, n) + group_name = choices[choice] + + # Victor: we are seeing very strange behavior on prod, where almost all users + # end up in the same group. Log at INFO to try to figure out what's going on. + log.info("DEBUG: adding user {0} to cohort {1}. choice={2}".format( + user, group_name,choice)) + group, created = CourseUserGroup.objects.get_or_create( course_id=course_id, group_type=CourseUserGroup.COHORT, name=group_name) - + user.course_groups.add(group) return group diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index efed39d536..88d9c1f508 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -6,7 +6,7 @@ from django.test.utils import override_settings from course_groups.models import CourseUserGroup from course_groups.cohorts import (get_cohort, get_course_cohorts, - is_commentable_cohorted) + is_commentable_cohorted, get_cohort_by_name) from xmodule.modulestore.django import modulestore, _MODULESTORES @@ -168,7 +168,7 @@ class TestCohorts(django.test.TestCase): self.assertEquals(get_cohort(user3, course.id), None, "No groups->no auto-cohorting") - + # Now make it different self.config_course_cohorts(course, [], cohorted=True, auto_cohort=True, @@ -180,6 +180,37 @@ class TestCohorts(django.test.TestCase): "user2 should still be in originally placed cohort") + def test_auto_cohorting_randomization(self): + """ + Make sure get_cohort() randomizes properly. + """ + course = modulestore().get_course("edX/toy/2012_Fall") + self.assertEqual(course.id, "edX/toy/2012_Fall") + self.assertFalse(course.is_cohorted) + + groups = ["group_{0}".format(n) for n in range(5)] + self.config_course_cohorts(course, [], cohorted=True, + auto_cohort=True, + auto_cohort_groups=groups) + + # Assign 100 users to cohorts + for i in range(100): + user = User.objects.create(username="test_{0}".format(i), + email="a@b{0}.com".format(i)) + get_cohort(user, course.id) + + # Now make sure that the assignment was at least vaguely random: + # each cohort should have at least 1, and fewer than 50 students. + # (with 5 groups, probability of 0 users in any group is about + # .8**100= 2.0e-10) + for cohort_name in groups: + cohort = get_cohort_by_name(course.id, cohort_name) + num_users = cohort.users.count() + self.assertGreater(num_users, 1) + self.assertLess(num_users, 50) + + + def test_get_course_cohorts(self): course1_id = 'a/b/c' course2_id = 'e/f/g' From bf0e5a270183b90becc56ec7f709deef7bb26fd6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 11 Mar 2013 21:14:24 -0400 Subject: [PATCH 07/17] do a bunch of pylint improvements --- .../contentstore/tests/test_contentstore.py | 159 +++++++++--------- 1 file changed, 78 insertions(+), 81 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 58197b9762..99ef1169b1 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -9,10 +9,8 @@ from tempdir import mkdtemp_clean import json from fs.osfs import OSFS import copy -from mock import Mock -from json import dumps, loads +from json import loads -from student.models import Registration from django.contrib.auth.models import User from cms.djangoapps.contentstore.utils import get_modulestore @@ -22,12 +20,11 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore import Location from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course -from xmodule.modulestore.django import modulestore, _MODULESTORES +from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.templates import update_templates from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -81,8 +78,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_static_tab_reordering(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # reverse the ordering reverse_tabs = [] @@ -90,9 +87,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): if tab['type'] == 'static_tab': reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) - resp = self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") + self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # compare to make sure that the tabs information is in the expected order after the server call course_tabs = [] @@ -105,28 +102,29 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') - sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None])) + sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) - chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) # make sure the parent no longer points to the child object which was deleted self.assertTrue(sequential.location.url() in chapter.definition['children']) - resp = self.client.post(reverse('delete_item'), json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), "application/json") + self.client.post(reverse('delete_item'), + json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), + "application/json") - bFound = False + found = False try: - sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None])) - bFound = True + module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) + found = True except ItemNotFoundError: pass - self.assertFalse(bFound) + self.assertFalse(found) - chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) # make sure the parent no longer points to the child object which was deleted self.assertFalse(sequential.location.url() in chapter.definition['children']) @@ -139,22 +137,22 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): while there is a base definition in /about/effort.html ''' import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None])) + module_store = modulestore('direct') + effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None])) self.assertEqual(effort.definition['data'], '6 hours') # this one should be in a non-override folder - effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) + effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) self.assertEqual(effort.definition['data'], 'TBD') def test_remove_hide_progress_tab(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - course = ms.get_item(source_location) + course = module_store.get_item(source_location) self.assertNotIn('hide_progress_tab', course.metadata) def test_clone_course(self): @@ -173,19 +171,19 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): data = parse_json(resp) self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course') - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course') - clone_course(ms, cs, source_location, dest_location) + clone_course(module_store, content_store, source_location, dest_location) # now loop through all the units in the course and verify that the clone can render them, which # means the objects are at least present - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) - clone_items = ms.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) + clone_items = module_store.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) self.assertGreater(len(clone_items), 0) for descriptor in items: new_loc = descriptor.location._replace(org='MITx', course='999') @@ -196,14 +194,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - delete_course(ms, cs, location, commit=True) + delete_course(module_store, content_store, location, commit=True) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertEqual(len(items), 0) def verify_content_existence(self, modulestore, root_dir, location, dirname, category_name, filename_suffix=''): @@ -218,10 +216,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertTrue(fs.exists(item.location.name + filename_suffix)) def test_export_course(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') root_dir = path(mkdtemp_clean()) @@ -229,43 +227,43 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(ms, cs, location, root_dir, 'test_export') + export_to_xml(module_store, content_store, location, root_dir, 'test_export') # check for static tabs - self.verify_content_existence(ms, root_dir, location, 'tabs', 'static_tab', '.html') + self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'info', 'course_info', '.html') + self.verify_content_existence(module_store, root_dir, location, 'info', 'course_info', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'custom_tags', 'custom_tag_template') + self.verify_content_existence(module_store, root_dir, location, 'custom_tags', 'custom_tag_template') # check for graiding_policy.json fs = OSFS(root_dir / 'test_export/policies/6.002_Spring_2012') self.assertTrue(fs.exists('grading_policy.json')) - course = ms.get_item(location) + course = module_store.get_item(location) # compare what's on disk compared to what we have in our course - with fs.open('grading_policy.json','r') as grading_policy: - on_disk = loads(grading_policy.read()) + with fs.open('grading_policy.json', 'r') as grading_policy: + on_disk = loads(grading_policy.read()) self.assertEqual(on_disk, course.definition['data']['grading_policy']) #check for policy.json self.assertTrue(fs.exists('policy.json')) # compare what's on disk to what we have in the course module - with fs.open('policy.json','r') as course_policy: + with fs.open('policy.json', 'r') as course_policy: on_disk = loads(course_policy.read()) self.assertIn('course/6.002_Spring_2012', on_disk) self.assertEqual(on_disk['course/6.002_Spring_2012'], course.metadata) # remove old course - delete_course(ms, cs, location) + delete_course(module_store, content_store, location) # reimport - import_from_xml(ms, root_dir, ['test_export']) + import_from_xml(module_store, root_dir, ['test_export']) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) for descriptor in items: print "Checking {0}....".format(descriptor.location.url()) @@ -275,11 +273,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): shutil.rmtree(root_dir) def test_course_handouts_rewrites(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() # import a test course - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) handout_location = Location(['i4x', 'edX', 'full', 'course_info', 'handouts']) @@ -294,32 +292,32 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf') def test_export_course_with_unknown_metadata(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') root_dir = path(mkdtemp_clean()) - course = ms.get_item(location) + course = module_store.get_item(location) # add a bool piece of unknown metadata so we can verify we don't throw an exception course.metadata['new_metadata'] = True - ms.update_metadata(location, course.metadata) + module_store.update_metadata(location, course.metadata) print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - bExported = False + exported = False try: - export_to_xml(ms, cs, location, root_dir, 'test_export') - bExported = True + export_to_xml(module_store, content_store, location, root_dir, 'test_export') + exported = True except Exception: pass - self.assertTrue(bExported) + self.assertTrue(exported) class ContentStoreTest(ModuleStoreTestCase): """ @@ -458,7 +456,7 @@ class ContentStoreTest(ModuleStoreTestCase): def test_capa_module(self): """Test that a problem treats markdown specially.""" - course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') + CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') problem_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', @@ -480,10 +478,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_import_metadata_with_attempts_empty_string(self): import_from_xml(modulestore(), 'common/test/data/', ['simple']) - ms = modulestore('direct') + module_store = modulestore('direct') did_load_item = False try: - ms.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) + module_store.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) did_load_item = True except ItemNotFoundError: pass @@ -494,10 +492,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_metadata_inheritance(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) - verticals = ms.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) + verticals = module_store.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: @@ -508,15 +506,15 @@ class ContentStoreTest(ModuleStoreTestCase): new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - + # crate a new module and add it as a child to a vertical - ms.clone_item(source_template_location, new_component_location) + module_store.clone_item(source_template_location, new_component_location) parent = verticals[0] - ms.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) + module_store.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) # flush the cache - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) # check for grace period definition which should be defined at the course level self.assertIn('graceperiod', new_module.metadata) @@ -529,11 +527,11 @@ class ContentStoreTest(ModuleStoreTestCase): # now let's define an override at the leaf node level # new_module.metadata['graceperiod'] = '1 day' - ms.update_metadata(new_module.location, new_module.metadata) + module_store.update_metadata(new_module.location, new_module.metadata) # flush the cache and refetch - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) self.assertIn('graceperiod', new_module.metadata) self.assertEqual('1 day', new_module.metadata['graceperiod']) @@ -542,15 +540,15 @@ class ContentStoreTest(ModuleStoreTestCase): class TemplateTestCase(ModuleStoreTestCase): def test_template_cleanup(self): - ms = modulestore('direct') + module_store = modulestore('direct') # insert a bogus template in the store bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - - ms.clone_item(source_template_location, bogus_template_location) - verify_create = ms.get_item(bogus_template_location) + module_store.clone_item(source_template_location, bogus_template_location) + + verify_create = module_store.get_item(bogus_template_location) self.assertIsNotNone(verify_create) # now run cleanup @@ -559,10 +557,9 @@ class TemplateTestCase(ModuleStoreTestCase): # now try to find dangling template, it should not be in DB any longer asserted = False try: - verify_create = ms.get_item(bogus_template_location) + verify_create = module_store.get_item(bogus_template_location) except ItemNotFoundError: asserted = True - self.assertTrue(asserted) - + self.assertTrue(asserted) From 554cb752fa0981aa527d23a1c565b2308692d37f Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 12 Mar 2013 12:20:11 -0400 Subject: [PATCH 08/17] Pep8 cleanup --- .../xmodule/combined_open_ended_module.py | 12 +- .../combined_open_ended_modulev1.py | 161 ++++++++------- .../combined_open_ended_rubric.py | 145 ++++++++------ .../controller_query_service.py | 14 +- .../grading_service_module.py | 13 +- .../open_ended_image_submission.py | 10 +- .../open_ended_module.py | 61 +++--- .../openendedchild.py | 43 ++-- .../peer_grading_service.py | 9 +- .../self_assessment_module.py | 10 +- .../xmodule/xmodule/peer_grading_module.py | 64 +++--- .../xmodule/tests/test_combined_open_ended.py | 183 +++++++++--------- .../xmodule/tests/test_self_assessment.py | 46 ++--- .../xmodule/tests/test_util_open_ended.py | 18 +- .../open_ended_notifications.py | 24 ++- .../open_ended_grading/staff_grading.py | 1 + .../staff_grading_service.py | 65 ++++--- lms/djangoapps/open_ended_grading/tests.py | 49 +++-- lms/djangoapps/open_ended_grading/views.py | 61 +++--- 19 files changed, 539 insertions(+), 450 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 659590b5b4..0cc69a4a24 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -10,7 +10,6 @@ from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import Comb log = logging.getLogger("mitx.courseware") - VERSION_TUPLES = ( ('1', CombinedOpenEndedV1Descriptor, CombinedOpenEndedV1Module), ) @@ -18,6 +17,7 @@ VERSION_TUPLES = ( DEFAULT_VERSION = 1 DEFAULT_VERSION = str(DEFAULT_VERSION) + class CombinedOpenEndedModule(XModule): """ This is a module that encapsulates all open ended grading (self assessment, peer assessment, etc). @@ -60,7 +60,7 @@ class CombinedOpenEndedModule(XModule): def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) + instance_state, shared_state, **kwargs) """ Definition file should have one or many task blocks, a rubric block, and a prompt block: @@ -129,13 +129,15 @@ class CombinedOpenEndedModule(XModule): version_index = versions.index(self.version) static_data = { - 'rewrite_content_links' : self.rewrite_content_links, + 'rewrite_content_links': self.rewrite_content_links, } self.child_descriptor = descriptors[version_index](self.system) - self.child_definition = descriptors[version_index].definition_from_xml(etree.fromstring(definition['data']), self.system) + self.child_definition = descriptors[version_index].definition_from_xml(etree.fromstring(definition['data']), + self.system) self.child_module = modules[version_index](self.system, location, self.child_definition, self.child_descriptor, - instance_state = json.dumps(instance_state), metadata = self.metadata, static_data= static_data) + instance_state=json.dumps(instance_state), metadata=self.metadata, + static_data=static_data) def get_html(self): return self.child_module.get_html() 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 5c3bfa5b2a..20cedaab75 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 @@ -40,14 +40,15 @@ ACCEPT_FILE_UPLOAD = False TRUE_DICT = ["True", True, "TRUE", "true"] HUMAN_TASK_TYPE = { - 'selfassessment' : "Self Assessment", - 'openended' : "edX Assessment", - } + 'selfassessment': "Self Assessment", + 'openended': "edX Assessment", +} #Default value that controls whether or not to skip basic spelling checks in the controller #Metadata overrides this SKIP_BASIC_CHECKS = False + class CombinedOpenEndedV1Module(): """ This is a module that encapsulates all open ended grading (self assessment, peer assessment, etc). @@ -83,7 +84,7 @@ class CombinedOpenEndedV1Module(): TEMPLATE_DIR = "combinedopenended" def __init__(self, system, location, definition, descriptor, - instance_state=None, shared_state=None, metadata = None, static_data = None, **kwargs): + instance_state=None, shared_state=None, metadata=None, static_data=None, **kwargs): """ Definition file should have one or many task blocks, a rubric block, and a prompt block: @@ -122,7 +123,7 @@ class CombinedOpenEndedV1Module(): self.metadata = metadata self.display_name = metadata.get('display_name', "Open Ended") - self.rewrite_content_links = static_data.get('rewrite_content_links',"") + self.rewrite_content_links = static_data.get('rewrite_content_links', "") # Load instance state @@ -152,10 +153,10 @@ class CombinedOpenEndedV1Module(): self.skip_basic_checks = self.metadata.get('skip_spelling_checks', SKIP_BASIC_CHECKS) display_due_date_string = self.metadata.get('due', None) - + grace_period_string = self.metadata.get('graceperiod', None) try: - self.timeinfo = TimeInfo(display_due_date_string, grace_period_string) + self.timeinfo = TimeInfo(display_due_date_string, grace_period_string) except: log.error("Error parsing due date information in location {0}".format(location)) raise @@ -177,10 +178,10 @@ class CombinedOpenEndedV1Module(): 'rubric': definition['rubric'], 'display_name': self.display_name, 'accept_file_upload': self.accept_file_upload, - 'close_date' : self.timeinfo.close_date, - 's3_interface' : self.system.s3_interface, - 'skip_basic_checks' : self.skip_basic_checks, - } + 'close_date': self.timeinfo.close_date, + 's3_interface': self.system.s3_interface, + 'skip_basic_checks': self.skip_basic_checks, + } self.task_xml = definition['task_xml'] self.location = location @@ -223,15 +224,15 @@ class CombinedOpenEndedV1Module(): child_modules = { 'openended': open_ended_module.OpenEndedModule, 'selfassessment': self_assessment_module.SelfAssessmentModule, - } + } child_descriptors = { 'openended': open_ended_module.OpenEndedDescriptor, 'selfassessment': self_assessment_module.SelfAssessmentDescriptor, - } + } children = { 'modules': child_modules, 'descriptors': child_descriptors, - } + } return children def setup_next_task(self, reset=False): @@ -267,7 +268,8 @@ class CombinedOpenEndedV1Module(): self.current_task_parsed_xml = self.current_task_descriptor.definition_from_xml(etree_xml, self.system) if current_task_state is None and self.current_task_number == 0: self.current_task = child_task_module(self.system, self.location, - self.current_task_parsed_xml, self.current_task_descriptor, self.static_data) + self.current_task_parsed_xml, self.current_task_descriptor, + self.static_data) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING elif current_task_state is None and self.current_task_number > 0: @@ -280,18 +282,20 @@ class CombinedOpenEndedV1Module(): 'attempts': 0, 'created': True, 'history': [{'answer': last_response}], - }) + }) self.current_task = child_task_module(self.system, self.location, - self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - instance_state=current_task_state) + self.current_task_parsed_xml, self.current_task_descriptor, + self.static_data, + instance_state=current_task_state) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING else: if self.current_task_number > 0 and not reset: current_task_state = self.overwrite_state(current_task_state) self.current_task = child_task_module(self.system, self.location, - self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - instance_state=current_task_state) + self.current_task_parsed_xml, self.current_task_descriptor, + self.static_data, + instance_state=current_task_state) return True @@ -307,8 +311,8 @@ class CombinedOpenEndedV1Module(): last_response_data = self.get_last_response(self.current_task_number - 1) current_response_data = self.get_current_attributes(self.current_task_number) - if(current_response_data['min_score_to_attempt'] > last_response_data['score'] - or current_response_data['max_score_to_attempt'] < last_response_data['score']): + if (current_response_data['min_score_to_attempt'] > last_response_data['score'] + or current_response_data['max_score_to_attempt'] < last_response_data['score']): self.state = self.DONE self.allow_reset = True @@ -334,8 +338,8 @@ class CombinedOpenEndedV1Module(): 'display_name': self.display_name, 'accept_file_upload': self.accept_file_upload, 'location': self.location, - 'legend_list' : LEGEND_LIST, - } + 'legend_list': LEGEND_LIST, + } return context @@ -404,7 +408,7 @@ class CombinedOpenEndedV1Module(): task_parsed_xml = task_descriptor.definition_from_xml(etree_xml, self.system) task = children['modules'][task_type](self.system, self.location, task_parsed_xml, task_descriptor, - self.static_data, instance_state=task_state) + self.static_data, instance_state=task_state) last_response = task.latest_answer() last_score = task.latest_score() last_post_assessment = task.latest_post_assessment(self.system) @@ -426,10 +430,10 @@ class CombinedOpenEndedV1Module(): rubric_scores = rubric_data['rubric_scores'] grader_types = rubric_data['grader_types'] feedback_items = rubric_data['feedback_items'] - feedback_dicts = rubric_data['feedback_dicts'] + feedback_dicts = rubric_data['feedback_dicts'] grader_ids = rubric_data['grader_ids'] - submission_ids = rubric_data['submission_ids'] - elif task_type== "selfassessment": + submission_ids = rubric_data['submission_ids'] + elif task_type == "selfassessment": rubric_scores = last_post_assessment grader_types = ['SA'] feedback_items = [''] @@ -446,7 +450,7 @@ class CombinedOpenEndedV1Module(): human_state = task.HUMAN_NAMES[state] else: human_state = state - if len(grader_types)>0: + if len(grader_types) > 0: grader_type = grader_types[0] else: grader_type = "IN" @@ -468,15 +472,15 @@ class CombinedOpenEndedV1Module(): 'correct': last_correctness, 'min_score_to_attempt': min_score_to_attempt, 'max_score_to_attempt': max_score_to_attempt, - 'rubric_scores' : rubric_scores, - 'grader_types' : grader_types, - 'feedback_items' : feedback_items, - 'grader_type' : grader_type, - 'human_grader_type' : human_grader_name, - 'feedback_dicts' : feedback_dicts, - 'grader_ids' : grader_ids, - 'submission_ids' : submission_ids, - } + 'rubric_scores': rubric_scores, + 'grader_types': grader_types, + 'feedback_items': feedback_items, + 'grader_type': grader_type, + 'human_grader_type': human_grader_name, + 'feedback_dicts': feedback_dicts, + 'grader_ids': grader_ids, + 'submission_ids': submission_ids, + } return last_response_dict def update_task_states(self): @@ -519,20 +523,27 @@ class CombinedOpenEndedV1Module(): Output: Dictionary to be rendered via ajax that contains the result html. """ all_responses = [] - loop_up_to_task = self.current_task_number+1 - for i in xrange(0,loop_up_to_task): + loop_up_to_task = self.current_task_number + 1 + for i in xrange(0, loop_up_to_task): all_responses.append(self.get_last_response(i)) - rubric_scores = [all_responses[i]['rubric_scores'] for i in xrange(0,len(all_responses)) if len(all_responses[i]['rubric_scores'])>0 and all_responses[i]['grader_types'][0] in HUMAN_GRADER_TYPE.keys()] - grader_types = [all_responses[i]['grader_types'] for i in xrange(0,len(all_responses)) if len(all_responses[i]['grader_types'])>0 and all_responses[i]['grader_types'][0] in HUMAN_GRADER_TYPE.keys()] - feedback_items = [all_responses[i]['feedback_items'] for i in xrange(0,len(all_responses)) if len(all_responses[i]['feedback_items'])>0 and all_responses[i]['grader_types'][0] in HUMAN_GRADER_TYPE.keys()] - rubric_html = self.rubric_renderer.render_combined_rubric(stringify_children(self.static_data['rubric']), rubric_scores, - grader_types, feedback_items) + rubric_scores = [all_responses[i]['rubric_scores'] for i in xrange(0, len(all_responses)) if + len(all_responses[i]['rubric_scores']) > 0 and all_responses[i]['grader_types'][ + 0] in HUMAN_GRADER_TYPE.keys()] + grader_types = [all_responses[i]['grader_types'] for i in xrange(0, len(all_responses)) if + len(all_responses[i]['grader_types']) > 0 and all_responses[i]['grader_types'][ + 0] in HUMAN_GRADER_TYPE.keys()] + feedback_items = [all_responses[i]['feedback_items'] for i in xrange(0, len(all_responses)) if + len(all_responses[i]['feedback_items']) > 0 and all_responses[i]['grader_types'][ + 0] in HUMAN_GRADER_TYPE.keys()] + rubric_html = self.rubric_renderer.render_combined_rubric(stringify_children(self.static_data['rubric']), + rubric_scores, + grader_types, feedback_items) response_dict = all_responses[-1] context = { 'results': rubric_html, - 'task_name' : 'Scored Rubric', - 'class_name' : 'combined-rubric-container' + 'task_name': 'Scored Rubric', + 'class_name': 'combined-rubric-container' } html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} @@ -544,8 +555,8 @@ class CombinedOpenEndedV1Module(): Output: Dictionary to be rendered via ajax that contains the result html. """ context = { - 'legend_list' : LEGEND_LIST, - } + 'legend_list': LEGEND_LIST, + } html = self.system.render_template('{0}/combined_open_ended_legend.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} @@ -556,15 +567,16 @@ class CombinedOpenEndedV1Module(): Output: Dictionary to be rendered via ajax that contains the result html. """ self.update_task_states() - loop_up_to_task = self.current_task_number+1 - all_responses =[] - for i in xrange(0,loop_up_to_task): + loop_up_to_task = self.current_task_number + 1 + all_responses = [] + for i in xrange(0, loop_up_to_task): all_responses.append(self.get_last_response(i)) context_list = [] for ri in all_responses: - for i in xrange(0,len(ri['rubric_scores'])): - feedback = ri['feedback_dicts'][i].get('feedback','') - rubric_data = self.rubric_renderer.render_rubric(stringify_children(self.static_data['rubric']), ri['rubric_scores'][i]) + for i in xrange(0, len(ri['rubric_scores'])): + feedback = ri['feedback_dicts'][i].get('feedback', '') + rubric_data = self.rubric_renderer.render_rubric(stringify_children(self.static_data['rubric']), + ri['rubric_scores'][i]) if rubric_data['success']: rubric_html = rubric_data['html'] else: @@ -572,23 +584,23 @@ class CombinedOpenEndedV1Module(): context = { 'rubric_html': rubric_html, 'grader_type': ri['grader_type'], - 'feedback' : feedback, - 'grader_id' : ri['grader_ids'][i], - 'submission_id' : ri['submission_ids'][i], + 'feedback': feedback, + 'grader_id': ri['grader_ids'][i], + 'submission_id': ri['submission_ids'][i], } context_list.append(context) feedback_table = self.system.render_template('{0}/open_ended_result_table.html'.format(self.TEMPLATE_DIR), { - 'context_list' : context_list, - 'grader_type_image_dict' : GRADER_TYPE_IMAGE_DICT, - 'human_grader_types' : HUMAN_GRADER_TYPE, + 'context_list': context_list, + 'grader_type_image_dict': GRADER_TYPE_IMAGE_DICT, + 'human_grader_types': HUMAN_GRADER_TYPE, 'rows': 50, 'cols': 50, }) context = { 'results': feedback_table, - 'task_name' : "Feedback", - 'class_name' : "result-container", - } + 'task_name': "Feedback", + 'class_name': "result-container", + } html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} @@ -617,8 +629,8 @@ class CombinedOpenEndedV1Module(): 'reset': self.reset, 'get_results': self.get_results, 'get_combined_rubric': self.get_rubric, - 'get_status' : self.get_status_ajax, - 'get_legend' : self.get_legend, + 'get_status': self.get_status_ajax, + 'get_legend': self.get_legend, } if dispatch not in handlers: @@ -681,7 +693,7 @@ class CombinedOpenEndedV1Module(): 'task_states': self.task_states, 'attempts': self.attempts, 'ready_to_reset': self.allow_reset, - } + } return json.dumps(state) @@ -699,11 +711,12 @@ class CombinedOpenEndedV1Module(): context = { 'status_list': status, - 'grader_type_image_dict' : GRADER_TYPE_IMAGE_DICT, - 'legend_list' : LEGEND_LIST, - 'render_via_ajax' : render_via_ajax, + 'grader_type_image_dict': GRADER_TYPE_IMAGE_DICT, + 'legend_list': LEGEND_LIST, + 'render_via_ajax': render_via_ajax, } - status_html = self.system.render_template("{0}/combined_open_ended_status.html".format(self.TEMPLATE_DIR), context) + status_html = self.system.render_template("{0}/combined_open_ended_status.html".format(self.TEMPLATE_DIR), + context) return status_html @@ -736,7 +749,7 @@ class CombinedOpenEndedV1Module(): score_dict = { 'score': score, 'total': max_score, - } + } return score_dict @@ -793,7 +806,9 @@ class CombinedOpenEndedV1Descriptor(XmlDescriptor, EditingDescriptor): for child in expected_children: if len(xml_object.xpath(child)) == 0: #This is a staff_facing_error - raise ValueError("Combined Open Ended definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format(child)) + raise ValueError( + "Combined Open Ended definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format( + child)) def parse_task(k): """Assumes that xml_object has child k""" diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py index 8d1bd376fb..f4ea7648a1 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py @@ -4,24 +4,26 @@ from lxml import etree log = logging.getLogger(__name__) GRADER_TYPE_IMAGE_DICT = { - 'SA' : '/static/images/self_assessment_icon.png', - 'PE' : '/static/images/peer_grading_icon.png', - 'ML' : '/static/images/ml_grading_icon.png', - 'IN' : '/static/images/peer_grading_icon.png', - 'BC' : '/static/images/ml_grading_icon.png', - } + 'SA': '/static/images/self_assessment_icon.png', + 'PE': '/static/images/peer_grading_icon.png', + 'ML': '/static/images/ml_grading_icon.png', + 'IN': '/static/images/peer_grading_icon.png', + 'BC': '/static/images/ml_grading_icon.png', +} HUMAN_GRADER_TYPE = { - 'SA' : 'Self-Assessment', - 'PE' : 'Peer-Assessment', - 'IN' : 'Instructor-Assessment', - 'ML' : 'AI-Assessment', - 'BC' : 'AI-Assessment', - } + 'SA': 'Self-Assessment', + 'PE': 'Peer-Assessment', + 'IN': 'Instructor-Assessment', + 'ML': 'AI-Assessment', + 'BC': 'AI-Assessment', +} DO_NOT_DISPLAY = ['BC', 'IN'] -LEGEND_LIST = [{'name' : HUMAN_GRADER_TYPE[k], 'image' : GRADER_TYPE_IMAGE_DICT[k]} for k in GRADER_TYPE_IMAGE_DICT.keys() if k not in DO_NOT_DISPLAY ] +LEGEND_LIST = [{'name': HUMAN_GRADER_TYPE[k], 'image': GRADER_TYPE_IMAGE_DICT[k]} for k in GRADER_TYPE_IMAGE_DICT.keys() + if k not in DO_NOT_DISPLAY] + class RubricParsingError(Exception): def __init__(self, msg): @@ -29,15 +31,14 @@ class RubricParsingError(Exception): class CombinedOpenEndedRubric(object): - TEMPLATE_DIR = "combinedopenended/openended" - def __init__ (self, system, view_only = False): + def __init__(self, system, view_only=False): self.has_score = False self.view_only = view_only self.system = system - def render_rubric(self, rubric_xml, score_list = None): + def render_rubric(self, rubric_xml, score_list=None): ''' render_rubric: takes in an xml string and outputs the corresponding html for that xml, given the type of rubric we're generating @@ -50,11 +51,11 @@ class CombinedOpenEndedRubric(object): success = False try: rubric_categories = self.extract_categories(rubric_xml) - if score_list and len(score_list)==len(rubric_categories): - for i in xrange(0,len(rubric_categories)): + if score_list and len(score_list) == len(rubric_categories): + for i in xrange(0, len(rubric_categories)): category = rubric_categories[i] - for j in xrange(0,len(category['options'])): - if score_list[i]==j: + for j in xrange(0, len(category['options'])): + if score_list[i] == j: rubric_categories[i]['options'][j]['selected'] = True rubric_scores = [cat['score'] for cat in rubric_categories] max_scores = map((lambda cat: cat['options'][-1]['points']), rubric_categories) @@ -63,19 +64,20 @@ class CombinedOpenEndedRubric(object): if self.view_only: rubric_template = '{0}/open_ended_view_only_rubric.html'.format(self.TEMPLATE_DIR) html = self.system.render_template(rubric_template, - {'categories': rubric_categories, - 'has_score': self.has_score, - 'view_only': self.view_only, - 'max_score': max_score, - 'combined_rubric' : False - }) + {'categories': rubric_categories, + 'has_score': self.has_score, + 'view_only': self.view_only, + 'max_score': max_score, + 'combined_rubric': False + }) success = True except: #This is a staff_facing_error - error_message = "[render_rubric] Could not parse the rubric with xml: {0}. Contact the learning sciences group for assistance.".format(rubric_xml) + error_message = "[render_rubric] Could not parse the rubric with xml: {0}. Contact the learning sciences group for assistance.".format( + rubric_xml) log.exception(error_message) raise RubricParsingError(error_message) - return {'success' : success, 'html' : html, 'rubric_scores' : rubric_scores} + return {'success': success, 'html': html, 'rubric_scores': rubric_scores} def check_if_rubric_is_parseable(self, rubric_string, location, max_score_allowed, max_score): rubric_dict = self.render_rubric(rubric_string) @@ -83,7 +85,8 @@ class CombinedOpenEndedRubric(object): rubric_feedback = rubric_dict['html'] if not success: #This is a staff_facing_error - error_message = "Could not parse rubric : {0} for location {1}. Contact the learning sciences group for assistance.".format(rubric_string, location.url()) + error_message = "Could not parse rubric : {0} for location {1}. Contact the learning sciences group for assistance.".format( + rubric_string, location.url()) log.error(error_message) raise RubricParsingError(error_message) @@ -101,7 +104,7 @@ class CombinedOpenEndedRubric(object): if total != max_score: #This is a staff_facing_error error_msg = "The max score {0} for problem {1} does not match the total number of points in the rubric {2}. Contact the learning sciences group for assistance.".format( - max_score, location, total) + max_score, location, total) log.error(error_msg) raise RubricParsingError(error_msg) @@ -123,7 +126,9 @@ class CombinedOpenEndedRubric(object): for category in element: if category.tag != 'category': #This is a staff_facing_error - raise RubricParsingError("[extract_categories] Expected a tag: got {0} instead. Contact the learning sciences group for assistance.".format(category.tag)) + raise RubricParsingError( + "[extract_categories] Expected a tag: got {0} instead. Contact the learning sciences group for assistance.".format( + category.tag)) else: categories.append(self.extract_category(category)) return categories @@ -150,13 +155,17 @@ class CombinedOpenEndedRubric(object): # if we are missing the score tag and we are expecting one elif self.has_score: #This is a staff_facing_error - raise RubricParsingError("[extract_category] Category {0} is missing a score. Contact the learning sciences group for assistance.".format(descriptionxml.text)) + raise RubricParsingError( + "[extract_category] Category {0} is missing a score. Contact the learning sciences group for assistance.".format( + descriptionxml.text)) # parse description if descriptionxml.tag != 'description': #This is a staff_facing_error - raise RubricParsingError("[extract_category]: expected description tag, got {0} instead. Contact the learning sciences group for assistance.".format(descriptionxml.tag)) + raise RubricParsingError( + "[extract_category]: expected description tag, got {0} instead. Contact the learning sciences group for assistance.".format( + descriptionxml.tag)) description = descriptionxml.text @@ -167,7 +176,9 @@ class CombinedOpenEndedRubric(object): for option in optionsxml: if option.tag != 'option': #This is a staff_facing_error - raise RubricParsingError("[extract_category]: expected option tag, got {0} instead. Contact the learning sciences group for assistance.".format(option.tag)) + raise RubricParsingError( + "[extract_category]: expected option tag, got {0} instead. Contact the learning sciences group for assistance.".format( + option.tag)) else: pointstr = option.get("points") if pointstr: @@ -177,13 +188,16 @@ class CombinedOpenEndedRubric(object): points = int(pointstr) except ValueError: #This is a staff_facing_error - raise RubricParsingError("[extract_category]: expected points to have int, got {0} instead. Contact the learning sciences group for assistance.".format(pointstr)) + raise RubricParsingError( + "[extract_category]: expected points to have int, got {0} instead. Contact the learning sciences group for assistance.".format( + pointstr)) elif autonumbering: # use the generated one if we're in the right mode points = cur_points cur_points = cur_points + 1 else: - raise Exception("[extract_category]: missing points attribute. Cannot continue to auto-create points values after a points value is explicitly defined.") + raise Exception( + "[extract_category]: missing points attribute. Cannot continue to auto-create points values after a points value is explicitly defined.") selected = score == points optiontext = option.text @@ -193,31 +207,32 @@ class CombinedOpenEndedRubric(object): options = sorted(options, key=lambda option: option['points']) CombinedOpenEndedRubric.validate_options(options) - return {'description': description, 'options': options, 'score' : score} + return {'description': description, 'options': options, 'score': score} - def render_combined_rubric(self,rubric_xml,scores,score_types,feedback_types): - success, score_tuples = CombinedOpenEndedRubric.reformat_scores_for_rendering(scores,score_types,feedback_types) + def render_combined_rubric(self, rubric_xml, scores, score_types, feedback_types): + success, score_tuples = CombinedOpenEndedRubric.reformat_scores_for_rendering(scores, score_types, + feedback_types) rubric_categories = self.extract_categories(rubric_xml) max_scores = map((lambda cat: cat['options'][-1]['points']), rubric_categories) max_score = max(max_scores) - for i in xrange(0,len(rubric_categories)): + for i in xrange(0, len(rubric_categories)): category = rubric_categories[i] - for j in xrange(0,len(category['options'])): + for j in xrange(0, len(category['options'])): rubric_categories[i]['options'][j]['grader_types'] = [] for tuple in score_tuples: - if tuple[1] == i and tuple[2] ==j: + if tuple[1] == i and tuple[2] == j: for grader_type in tuple[3]: rubric_categories[i]['options'][j]['grader_types'].append(grader_type) html = self.system.render_template('{0}/open_ended_combined_rubric.html'.format(self.TEMPLATE_DIR), - {'categories': rubric_categories, - 'has_score': True, - 'view_only': True, - 'max_score': max_score, - 'combined_rubric' : True, - 'grader_type_image_dict' : GRADER_TYPE_IMAGE_DICT, - 'human_grader_types' : HUMAN_GRADER_TYPE, - }) + {'categories': rubric_categories, + 'has_score': True, + 'view_only': True, + 'max_score': max_score, + 'combined_rubric': True, + 'grader_type_image_dict': GRADER_TYPE_IMAGE_DICT, + 'human_grader_types': HUMAN_GRADER_TYPE, + }) return html @@ -228,14 +243,16 @@ class CombinedOpenEndedRubric(object): ''' if len(options) == 0: #This is a staff_facing_error - raise RubricParsingError("[extract_category]: no options associated with this category. Contact the learning sciences group for assistance.") + raise RubricParsingError( + "[extract_category]: no options associated with this category. Contact the learning sciences group for assistance.") if len(options) == 1: return prev = options[0]['points'] for option in options[1:]: if prev == option['points']: #This is a staff_facing_error - raise RubricParsingError("[extract_category]: found duplicate point values between two different options. Contact the learning sciences group for assistance.") + raise RubricParsingError( + "[extract_category]: found duplicate point values between two different options. Contact the learning sciences group for assistance.") else: prev = option['points'] @@ -250,7 +267,7 @@ class CombinedOpenEndedRubric(object): @return: """ success = False - if len(scores)==0: + if len(scores) == 0: #This is a dev_facing_error log.error("Score length is 0 when trying to reformat rubric scores for rendering.") return success, "" @@ -264,25 +281,25 @@ class CombinedOpenEndedRubric(object): score_lists = [] score_type_list = [] feedback_type_list = [] - for i in xrange(0,len(scores)): + for i in xrange(0, len(scores)): score_cont_list = scores[i] - for j in xrange(0,len(score_cont_list)): + for j in xrange(0, len(score_cont_list)): score_list = score_cont_list[j] score_lists.append(score_list) score_type_list.append(score_types[i][j]) feedback_type_list.append(feedback_types[i][j]) score_list_len = len(score_lists[0]) - for i in xrange(0,len(score_lists)): + for i in xrange(0, len(score_lists)): score_list = score_lists[i] - if len(score_list)!=score_list_len: + if len(score_list) != score_list_len: return success, "" score_tuples = [] - for i in xrange(0,len(score_lists)): - for j in xrange(0,len(score_lists[i])): - tuple = [1,j,score_lists[i][j],[],[]] - score_tuples, tup_ind = CombinedOpenEndedRubric.check_for_tuple_matches(score_tuples,tuple) + for i in xrange(0, len(score_lists)): + for j in xrange(0, len(score_lists[i])): + tuple = [1, j, score_lists[i][j], [], []] + score_tuples, tup_ind = CombinedOpenEndedRubric.check_for_tuple_matches(score_tuples, tuple) score_tuples[tup_ind][0] += 1 score_tuples[tup_ind][3].append(score_type_list[i]) score_tuples[tup_ind][4].append(feedback_type_list[i]) @@ -302,14 +319,14 @@ class CombinedOpenEndedRubric(object): category = tuple[1] score = tuple[2] tup_ind = -1 - for t in xrange(0,len(tuples)): + for t in xrange(0, len(tuples)): if tuples[t][1] == category and tuples[t][2] == score: tup_ind = t break if tup_ind == -1: - tuples.append([0,category,score,[],[]]) - tup_ind = len(tuples)-1 + tuples.append([0, category, score, [], []]) + tup_ind = len(tuples) - 1 return tuples, tup_ind diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index 1dd5c57ad4..21715c8e57 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -8,6 +8,7 @@ class ControllerQueryService(GradingService): """ Interface to staff grading backend. """ + def __init__(self, config, system): config['system'] = system super(ControllerQueryService, self).__init__(config) @@ -59,7 +60,7 @@ class ControllerQueryService(GradingService): def get_flagged_problem_list(self, course_id): params = { 'course_id': course_id, - } + } response = self.get(self.flagged_problem_list_url, params) return response @@ -70,20 +71,21 @@ class ControllerQueryService(GradingService): 'student_id': student_id, 'submission_id': submission_id, 'action_type': action_type - } + } response = self.post(self.take_action_on_flags_url, params) return response + def convert_seconds_to_human_readable(seconds): if seconds < 60: human_string = "{0} seconds".format(seconds) elif seconds < 60 * 60: - human_string = "{0} minutes".format(round(seconds/60,1)) - elif seconds < (24*60*60): - human_string = "{0} hours".format(round(seconds/(60*60),1)) + human_string = "{0} minutes".format(round(seconds / 60, 1)) + elif seconds < (24 * 60 * 60): + human_string = "{0} hours".format(round(seconds / (60 * 60), 1)) else: - human_string = "{0} days".format(round(seconds/(60*60*24),1)) + human_string = "{0} days".format(round(seconds / (60 * 60 * 24), 1)) eta_string = "{0}".format(human_string) return eta_string diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py index 8a4caa1291..0f961794d5 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py @@ -19,6 +19,7 @@ class GradingService(object): """ Interface to staff grading backend. """ + def __init__(self, config): self.username = config['username'] self.password = config['password'] @@ -34,8 +35,8 @@ class GradingService(object): Returns the decoded json dict of the response. """ response = self.session.post(self.login_url, - {'username': self.username, - 'password': self.password, }) + {'username': self.username, + 'password': self.password, }) response.raise_for_status() @@ -47,7 +48,7 @@ class GradingService(object): """ try: op = lambda: self.session.post(url, data=data, - allow_redirects=allow_redirects) + allow_redirects=allow_redirects) r = self._try_with_login(op) except (RequestException, ConnectionError, HTTPError) as err: # reraise as promised GradingServiceError, but preserve stacktrace. @@ -63,8 +64,8 @@ class GradingService(object): """ log.debug(params) op = lambda: self.session.get(url, - allow_redirects=allow_redirects, - params=params) + allow_redirects=allow_redirects, + params=params) try: r = self._try_with_login(op) except (RequestException, ConnectionError, HTTPError) as err: @@ -92,7 +93,7 @@ class GradingService(object): r = self._login() if r and not r.get('success'): log.warning("Couldn't log into staff_grading backend. Response: %s", - r) + r) # try again response = operation() response.raise_for_status() diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py index edae69854f..6956f336a5 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py @@ -5,6 +5,7 @@ to send them to S3. try: from PIL import Image + ENABLE_PIL = True except: ENABLE_PIL = False @@ -51,6 +52,7 @@ class ImageProperties(object): """ Class to check properties of an image and to validate if they are allowed. """ + def __init__(self, image_data): """ Initializes class variables @@ -92,7 +94,7 @@ class ImageProperties(object): g = rgb[1] b = rgb[2] check_r = (r > 60) - check_g = (r * 0.4) < g < (r * 0.85) + check_g = (r * 0.4) < g < (r * 0.85) check_b = (r * 0.2) < b < (r * 0.7) colors_okay = check_r and check_b and check_g except: @@ -141,6 +143,7 @@ class URLProperties(object): Checks to see if a URL points to acceptable content. Added to check if students are submitting reasonable links to the peer grading image functionality of the external grading service. """ + def __init__(self, url_string): self.url_string = url_string @@ -212,7 +215,7 @@ def run_image_tests(image): success = image_properties.run_tests() except: log.exception("Cannot run image tests in combined open ended xmodule. May be an issue with a particular image," - "or an issue with the deployment configuration of PIL/Pillow") + "or an issue with the deployment configuration of PIL/Pillow") return success @@ -252,7 +255,8 @@ def upload_to_s3(file_to_upload, keyname, s3_interface): return True, public_url except: #This is a dev_facing_error - error_message = "Could not connect to S3 to upload peer grading image. Trying to utilize bucket: {0}".format(bucketname.lower()) + error_message = "Could not connect to S3 to upload peer grading image. Trying to utilize bucket: {0}".format( + bucketname.lower()) log.error(error_message) return False, error_message diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index aa805a5290..b16e7f5313 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -10,7 +10,7 @@ import logging from lxml import etree import capa.xqueue_interface as xqueue_interface -from xmodule.capa_module import ComplexEncoder +from xmodule.capa_module import ComplexEncoder from xmodule.editing_module import EditingDescriptor from xmodule.progress import Progress from xmodule.stringify import stringify_children @@ -104,7 +104,9 @@ class OpenEndedModule(openendedchild.OpenEndedChild): # response types) except TypeError, ValueError: #This is a dev_facing_error - log.exception("Grader payload from external open ended grading server is not a json object! Object: {0}".format(grader_payload)) + log.exception( + "Grader payload from external open ended grading server is not a json object! Object: {0}".format( + grader_payload)) self.initial_display = find_with_default(oeparam, 'initial_display', '') self.answer = find_with_default(oeparam, 'answer_display', 'No answer given.') @@ -148,7 +150,9 @@ class OpenEndedModule(openendedchild.OpenEndedChild): for tag in ['feedback', 'submission_id', 'grader_id', 'score']: if tag not in survey_responses: #This is a student_facing_error - return {'success': False, 'msg': "Could not find needed tag {0} in the survey responses. Please try submitting again.".format(tag)} + return {'success': False, + 'msg': "Could not find needed tag {0} in the survey responses. Please try submitting again.".format( + tag)} try: submission_id = int(survey_responses['submission_id']) grader_id = int(survey_responses['grader_id']) @@ -188,7 +192,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): } (error, msg) = qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + body=json.dumps(contents)) #Convert error to a success value success = True @@ -222,8 +226,8 @@ class OpenEndedModule(openendedchild.OpenEndedChild): str(len(self.history))) xheader = xqueue_interface.make_xheader(lms_callback_url=system.xqueue['callback_url'], - lms_key=queuekey, - queue_name=self.queue_name) + lms_key=queuekey, + queue_name=self.queue_name) contents = self.payload.copy() @@ -241,7 +245,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): # Submit request. When successful, 'msg' is the prior length of the queue (error, msg) = qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + body=json.dumps(contents)) # State associated with the queueing request queuestate = {'key': queuekey, @@ -300,7 +304,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): # We want to display available feedback in a particular order. # This dictionary specifies which goes first--lower first. - priorities = { # These go at the start of the feedback + priorities = {# These go at the start of the feedback 'spelling': 0, 'grammar': 1, # needs to be after all the other feedback @@ -400,7 +404,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): if not response_items['success']: return system.render_template("{0}/open_ended_error.html".format(self.TEMPLATE_DIR), - {'errors': feedback}) + {'errors': feedback}) feedback_template = system.render_template("{0}/open_ended_feedback.html".format(self.TEMPLATE_DIR), { 'grader_type': response_items['grader_type'], @@ -437,13 +441,13 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'valid': False, 'score': 0, 'feedback': '', - 'rubric_scores' : [[0]], - 'grader_types' : [''], - 'feedback_items' : [''], - 'feedback_dicts' : [{}], - 'grader_ids' : [0], - 'submission_ids' : [0], - } + 'rubric_scores': [[0]], + 'grader_types': [''], + 'feedback_items': [''], + 'feedback_dicts': [{}], + 'grader_ids': [0], + 'submission_ids': [0], + } try: score_result = json.loads(score_msg) except (TypeError, ValueError): @@ -470,7 +474,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): log.error(error_message) fail['feedback'] = error_message return fail - #This is to support peer grading + #This is to support peer grading if isinstance(score_result['score'], list): feedback_items = [] rubric_scores = [] @@ -527,12 +531,12 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'valid': True, 'score': score, 'feedback': feedback, - 'rubric_scores' : rubric_scores, - 'grader_types' : grader_types, - 'feedback_items' : feedback_items, - 'feedback_dicts' : feedback_dicts, - 'grader_ids' : grader_ids, - 'submission_ids' : submission_ids, + 'rubric_scores': rubric_scores, + 'grader_types': grader_types, + 'feedback_items': feedback_items, + 'feedback_dicts': feedback_dicts, + 'grader_ids': grader_ids, + 'submission_ids': submission_ids, } def latest_post_assessment(self, system, short_feedback=False, join_feedback=True): @@ -545,7 +549,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return "" feedback_dict = self._parse_score_msg(self.history[-1].get('post_assessment', ""), system, - join_feedback=join_feedback) + join_feedback=join_feedback) if not short_feedback: return feedback_dict['feedback'] if feedback_dict['valid'] else '' if feedback_dict['valid']: @@ -585,7 +589,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): #This is a dev_facing_error log.error("Cannot find {0} in handlers in handle_ajax function for open_ended_module.py".format(dispatch)) #This is a dev_facing_error - return json.dumps({'error': 'Error handling action. Please try again.', 'success' : False}) + return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) before = self.get_progress() d = handlers[dispatch](get, system) @@ -679,7 +683,6 @@ class OpenEndedModule(openendedchild.OpenEndedChild): correct = "" previous_answer = self.initial_display - context = { 'prompt': self.prompt, 'previous_answer': previous_answer, @@ -692,7 +695,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'child_type': 'openended', 'correct': correct, 'accept_file_upload': self.accept_file_upload, - 'eta_message' : eta_string, + 'eta_message': eta_string, } html = system.render_template('{0}/open_ended.html'.format(self.TEMPLATE_DIR), context) return html @@ -723,7 +726,9 @@ class OpenEndedDescriptor(XmlDescriptor, EditingDescriptor): for child in ['openendedparam']: if len(xml_object.xpath(child)) != 1: #This is a staff_facing_error - raise ValueError("Open Ended definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format(child)) + raise ValueError( + "Open Ended definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format( + child)) def parse(k): """Assumes that xml_object has child k""" diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 31d26fd3c3..91dec5943d 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -74,7 +74,7 @@ class OpenEndedChild(object): 'done': 'Done', } - def __init__(self, system, location, definition, descriptor, static_data, + def __init__(self, system, location, definition, descriptor, static_data, instance_state=None, shared_state=None, **kwargs): # Load instance state if instance_state is not None: @@ -108,15 +108,14 @@ class OpenEndedChild(object): self._max_score = static_data['max_score'] if system.open_ended_grading_interface: self.peer_gs = PeerGradingService(system.open_ended_grading_interface, system) - self.controller_qs = controller_query_service.ControllerQueryService(system.open_ended_grading_interface,system) + self.controller_qs = controller_query_service.ControllerQueryService(system.open_ended_grading_interface, + system) else: self.peer_gs = MockPeerGradingService() - self.controller_qs = None - - + self.controller_qs = None self.system = system - + self.location_string = location try: self.location_string = self.location_string.url() @@ -152,7 +151,8 @@ class OpenEndedChild(object): return True, { 'success': False, #This is a student_facing_error - 'error': 'You have attempted this problem {0} times. You are allowed {1} attempts.'.format(self.attempts, self.max_attempts) + 'error': 'You have attempted this problem {0} times. You are allowed {1} attempts.'.format( + self.attempts, self.max_attempts) } else: return False, {} @@ -180,8 +180,8 @@ class OpenEndedChild(object): try: answer = autolink_html(answer) cleaner = Cleaner(style=True, links=True, add_nofollow=False, page_structure=True, safe_attrs_only=True, - host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, - whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) + host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, + whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) clean_html = cleaner.clean_html(answer) clean_html = re.sub(r'

$', '', re.sub(r'^

', '', clean_html)) except: @@ -282,7 +282,7 @@ class OpenEndedChild(object): """ #This is a dev_facing_error log.warning("Open ended child state out sync. state: %r, get: %r. %s", - self.state, get, msg) + self.state, get, msg) #This is a student_facing_error return {'success': False, 'error': 'The problem state got out-of-sync. Please try reloading the page.'} @@ -308,7 +308,7 @@ class OpenEndedChild(object): @return: Boolean correct. """ correct = False - if(isinstance(score, (int, long, float, complex))): + if (isinstance(score, (int, long, float, complex))): score_ratio = int(score) / float(self.max_score()) correct = (score_ratio >= 0.66) return correct @@ -342,7 +342,8 @@ class OpenEndedChild(object): try: image_data.seek(0) - success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key, self.s3_interface) + success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key, + self.s3_interface) except: log.exception("Could not upload image to S3.") @@ -404,9 +405,9 @@ class OpenEndedChild(object): #In this case, an image was submitted by the student, but the image could not be uploaded to S3. Likely #a config issue (development vs deployment). For now, just treat this as a "success" log.exception("Student AJAX post to combined open ended xmodule indicated that it contained an image, " - "but the image was not able to be uploaded to S3. This could indicate a config" - "issue with this deployment, but it could also indicate a problem with S3 or with the" - "student image itself.") + "but the image was not able to be uploaded to S3. This could indicate a config" + "issue with this deployment, but it could also indicate a problem with S3 or with the" + "student image itself.") overall_success = True elif not has_file_to_upload: #If there is no file to upload, probably the student has embedded the link in the answer text @@ -445,7 +446,7 @@ class OpenEndedChild(object): response = {} #This is a student_facing_error error_string = ("You need to peer grade {0} more in order to make another submission. " - "You have graded {1}, and {2} are required. You have made {3} successful peer grading submissions.") + "You have graded {1}, and {2} are required. You have made {3} successful peer grading submissions.") try: response = self.peer_gs.get_data_for_location(self.location_string, student_id) count_graded = response['count_graded'] @@ -454,16 +455,18 @@ class OpenEndedChild(object): success = True except: #This is a dev_facing_error - log.error("Could not contact external open ended graders for location {0} and student {1}".format(self.location_string,student_id)) + log.error("Could not contact external open ended graders for location {0} and student {1}".format( + self.location_string, student_id)) #This is a student_facing_error error_message = "Could not contact the graders. Please notify course staff." return success, allowed_to_submit, error_message - if count_graded>=count_required: + if count_graded >= count_required: return success, allowed_to_submit, "" else: allowed_to_submit = False #This is a student_facing_error - error_message = error_string.format(count_required-count_graded, count_graded, count_required, student_sub_count) + error_message = error_string.format(count_required - count_graded, count_graded, count_required, + student_sub_count) return success, allowed_to_submit, error_message def get_eta(self): @@ -478,7 +481,7 @@ class OpenEndedChild(object): success = response['success'] if isinstance(success, basestring): - success = (success.lower()=="true") + success = (success.lower() == "true") if success: eta = controller_query_service.convert_seconds_to_human_readable(response['eta']) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 42c54f0463..5daf1b83b5 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -14,6 +14,7 @@ class PeerGradingService(GradingService): """ Interface with the grading controller for peer grading """ + def __init__(self, config, system): config['system'] = system super(PeerGradingService, self).__init__(config) @@ -36,10 +37,11 @@ class PeerGradingService(GradingService): def get_next_submission(self, problem_location, grader_id): response = self.get(self.get_next_submission_url, - {'location': problem_location, 'grader_id': grader_id}) + {'location': problem_location, 'grader_id': grader_id}) return self.try_to_decode(self._render_rubric(response)) - def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, submission_flagged): + def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, + submission_flagged): data = {'grader_id': grader_id, 'submission_id': submission_id, 'score': score, @@ -89,6 +91,7 @@ class PeerGradingService(GradingService): pass return text + """ This is a mock peer grading service that can be used for unit tests without making actual service calls to the grading controller @@ -122,7 +125,7 @@ class MockPeerGradingService(object): 'max_score': 4}) def save_calibration_essay(self, problem_location, grader_id, - calibration_essay_id, submission_key, score, + calibration_essay_id, submission_key, score, feedback, rubric_scores): return {'success': True, 'actual_score': 2} diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index f4be426667..a25d81eeae 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -95,7 +95,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): #This is a dev_facing_error log.error("Cannot find {0} in handlers in handle_ajax function for open_ended_module.py".format(dispatch)) #This is a dev_facing_error - return json.dumps({'error': 'Error handling action. Please try again.', 'success' : False}) + return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) before = self.get_progress() d = handlers[dispatch](get, system) @@ -224,7 +224,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): try: score = int(get['assessment']) score_list = get.getlist('score_list[]') - for i in xrange(0,len(score_list)): + for i in xrange(0, len(score_list)): score_list[i] = int(score_list[i]) except ValueError: #This is a dev_facing_error @@ -268,7 +268,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): 'allow_reset': self._allow_reset()} def latest_post_assessment(self, system): - latest_post_assessment = super(SelfAssessmentModule, self).latest_post_assessment(system) + latest_post_assessment = super(SelfAssessmentModule, self).latest_post_assessment(system) try: rubric_scores = json.loads(latest_post_assessment) except: @@ -305,7 +305,9 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor): for child in expected_children: if len(xml_object.xpath(child)) != 1: #This is a staff_facing_error - raise ValueError("Self assessment definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format(child)) + raise ValueError( + "Self assessment definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format( + child)) def parse(k): """Assumes that xml_object has child k""" diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 1e52dcf070..2ea8ab0db5 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -5,7 +5,7 @@ from lxml import etree from datetime import datetime from pkg_resources import resource_string -from .capa_module import ComplexEncoder +from .capa_module import ComplexEncoder from .editing_module import EditingDescriptor from .stringify import stringify_children from .x_module import XModule @@ -34,7 +34,7 @@ class PeerGradingModule(XModule): resource_string(__name__, 'js/src/peergrading/peer_grading_problem.coffee'), resource_string(__name__, 'js/src/collapsible.coffee'), resource_string(__name__, 'js/src/javascript_loader.coffee'), - ]} + ]} js_module_name = "PeerGrading" css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]} @@ -42,7 +42,7 @@ class PeerGradingModule(XModule): def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) + instance_state, shared_state, **kwargs) # Load instance state if instance_state is not None: @@ -53,12 +53,11 @@ class PeerGradingModule(XModule): #We need to set the location here so the child modules can use it system.set('location', location) self.system = system - if(self.system.open_ended_grading_interface): + if (self.system.open_ended_grading_interface): self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system) else: self.peer_gs = MockPeerGradingService() - self.use_for_single_location = self.metadata.get('use_for_single_location', USE_FOR_SINGLE_LOCATION) if isinstance(self.use_for_single_location, basestring): self.use_for_single_location = (self.use_for_single_location in TRUE_DICT) @@ -83,14 +82,13 @@ class PeerGradingModule(XModule): grace_period_string = self.metadata.get('graceperiod', None) try: - self.timeinfo = TimeInfo(display_due_date_string, grace_period_string) + self.timeinfo = TimeInfo(display_due_date_string, grace_period_string) except: log.error("Error parsing due date information in location {0}".format(location)) raise self.display_due_date = self.timeinfo.display_due_date - self.ajax_url = self.system.ajax_url if not self.ajax_url.endswith("/"): self.ajax_url = self.ajax_url + "/" @@ -148,13 +146,13 @@ class PeerGradingModule(XModule): 'save_grade': self.save_grade, 'save_calibration_essay': self.save_calibration_essay, 'problem': self.peer_grading_problem, - } + } if dispatch not in handlers: #This is a dev_facing_error log.error("Cannot find {0} in handlers in handle_ajax function for open_ended_module.py".format(dispatch)) #This is a dev_facing_error - return json.dumps({'error': 'Error handling action. Please try again.', 'success' : False}) + return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) d = handlers[dispatch](get) @@ -191,9 +189,10 @@ class PeerGradingModule(XModule): except: success, response = self.query_data_for_location() if not success: - log.exception("No instance data found and could not get data from controller for loc {0} student {1}".format( - self.system.location.url(), self.system.anonymous_student_id - )) + log.exception( + "No instance data found and could not get data from controller for loc {0} student {1}".format( + self.system.location.url(), self.system.anonymous_student_id + )) return None count_graded = response['count_graded'] count_required = response['count_required'] @@ -204,7 +203,7 @@ class PeerGradingModule(XModule): score_dict = { 'score': int(count_graded >= count_required), 'total': self.max_grade, - } + } return score_dict @@ -253,7 +252,7 @@ class PeerGradingModule(XModule): .format(self.peer_gs.url, location, grader_id)) #This is a student_facing_error return {'success': False, - 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} + 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} def save_grade(self, get): """ @@ -271,7 +270,8 @@ class PeerGradingModule(XModule): error: if there was an error in the submission, this is the error message """ - required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]', 'submission_flagged']) + required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]', + 'submission_flagged']) success, message = self._check_required(get, required) if not success: return self._err_response(message) @@ -287,14 +287,14 @@ class PeerGradingModule(XModule): try: response = self.peer_gs.save_grade(location, grader_id, submission_id, - score, feedback, submission_key, rubric_scores, submission_flagged) + score, feedback, submission_key, rubric_scores, submission_flagged) return response except GradingServiceError: #This is a dev_facing_error log.exception("""Error saving grade to open ended grading service. server url: {0}, location: {1}, submission_id:{2}, submission_key: {3}, score: {4}""" .format(self.peer_gs.url, - location, submission_id, submission_key, score) + location, submission_id, submission_key, score) ) #This is a student_facing_error return { @@ -382,7 +382,7 @@ class PeerGradingModule(XModule): .format(self.peer_gs.url, location)) #This is a student_facing_error return {'success': False, - 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} + 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} # if we can't parse the rubric into HTML, except etree.XMLSyntaxError: #This is a dev_facing_error @@ -390,7 +390,7 @@ class PeerGradingModule(XModule): .format(rubric)) #This is a student_facing_error return {'success': False, - 'error': 'Error displaying submission. Please notify course staff.'} + 'error': 'Error displaying submission. Please notify course staff.'} def save_calibration_essay(self, get): @@ -426,11 +426,13 @@ class PeerGradingModule(XModule): try: response = self.peer_gs.save_calibration_essay(location, grader_id, calibration_essay_id, - submission_key, score, feedback, rubric_scores) + submission_key, score, feedback, rubric_scores) return response except GradingServiceError: #This is a dev_facing_error - log.exception("Error saving calibration grade, location: {0}, submission_id: {1}, submission_key: {2}, grader_id: {3}".format(location, submission_id, submission_key, grader_id)) + log.exception( + "Error saving calibration grade, location: {0}, submission_id: {1}, submission_key: {2}, grader_id: {3}".format( + location, submission_id, submission_key, grader_id)) #This is a student_facing_error return self._err_response('There was an error saving your score. Please notify course staff.') @@ -440,7 +442,7 @@ class PeerGradingModule(XModule): ''' html = self.system.render_template('peer_grading/peer_grading_closed.html', { 'use_for_single_location': self.use_for_single_location - }) + }) return html @@ -503,12 +505,11 @@ class PeerGradingModule(XModule): problem['closed'] = True else: problem['closed'] = False - else: - # if we can't find the due date, assume that it doesn't have one + else: + # if we can't find the due date, assume that it doesn't have one problem['due'] = None problem['closed'] = False - ajax_url = self.ajax_url html = self.system.render_template('peer_grading/peer_grading.html', { 'course_id': self.system.course_id, @@ -519,7 +520,7 @@ class PeerGradingModule(XModule): # Checked above 'staff_access': False, 'use_single_location': self.use_for_single_location, - }) + }) return html @@ -531,7 +532,8 @@ class PeerGradingModule(XModule): if not self.use_for_single_location: #This is an error case, because it must be set to use a single location to be called without get parameters #This is a dev_facing_error - log.error("Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.") + log.error( + "Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.") return {'html': "", 'success': False} problem_location = self.link_to_location @@ -547,7 +549,7 @@ class PeerGradingModule(XModule): # Checked above 'staff_access': False, 'use_single_location': self.use_for_single_location, - }) + }) return {'html': html, 'success': True} @@ -560,7 +562,7 @@ class PeerGradingModule(XModule): state = { 'student_data_for_location': self.student_data_for_location, - } + } return json.dumps(state) @@ -596,7 +598,9 @@ class PeerGradingDescriptor(XmlDescriptor, EditingDescriptor): for child in expected_children: if len(xml_object.xpath(child)) == 0: #This is a staff_facing_error - raise ValueError("Peer grading definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format(child)) + raise ValueError( + "Peer grading definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format( + child)) def parse_task(k): """Assumes that xml_object has child k""" 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 a524ac2fd9..8a14e03ded 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -14,6 +14,7 @@ from datetime import datetime from . import test_system import test_util_open_ended + """ Tests for the various pieces of the CombinedOpenEndedGrading system @@ -39,41 +40,37 @@ class OpenEndedChildTest(unittest.TestCase): max_score = 1 static_data = { - 'max_attempts': 20, - 'prompt': prompt, - 'rubric': rubric, - 'max_score': max_score, - 'display_name': 'Name', - 'accept_file_upload': False, - 'close_date': None, - 's3_interface' : "", - 'open_ended_grading_interface' : {}, - 'skip_basic_checks' : False, - } + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + 'display_name': 'Name', + 'accept_file_upload': False, + 'close_date': None, + 's3_interface': "", + 'open_ended_grading_interface': {}, + 'skip_basic_checks': False, + } definition = Mock() descriptor = Mock() def setUp(self): self.test_system = test_system() self.openendedchild = OpenEndedChild(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) - + self.definition, self.descriptor, self.static_data, self.metadata) def test_latest_answer_empty(self): answer = self.openendedchild.latest_answer() self.assertEqual(answer, "") - def test_latest_score_empty(self): answer = self.openendedchild.latest_score() self.assertEqual(answer, None) - def test_latest_post_assessment_empty(self): answer = self.openendedchild.latest_post_assessment(self.test_system) self.assertEqual(answer, "") - def test_new_history_entry(self): new_answer = "New Answer" self.openendedchild.new_history_entry(new_answer) @@ -99,7 +96,6 @@ class OpenEndedChildTest(unittest.TestCase): score = self.openendedchild.latest_score() self.assertEqual(score, 4) - def test_record_latest_post_assessment(self): new_answer = "New Answer" self.openendedchild.new_history_entry(new_answer) @@ -107,7 +103,7 @@ class OpenEndedChildTest(unittest.TestCase): post_assessment = "Post assessment" self.openendedchild.record_latest_post_assessment(post_assessment) self.assertEqual(post_assessment, - self.openendedchild.latest_post_assessment(self.test_system)) + self.openendedchild.latest_post_assessment(self.test_system)) def test_get_score(self): new_answer = "New Answer" @@ -124,24 +120,22 @@ class OpenEndedChildTest(unittest.TestCase): self.assertEqual(score['score'], new_score) self.assertEqual(score['total'], self.static_data['max_score']) - def test_reset(self): self.openendedchild.reset(self.test_system) state = json.loads(self.openendedchild.get_instance_state()) self.assertEqual(state['state'], OpenEndedChild.INITIAL) - def test_is_last_response_correct(self): new_answer = "New Answer" self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(self.static_data['max_score']) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'correct') + 'correct') self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(0) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'incorrect') + 'incorrect') class OpenEndedModuleTest(unittest.TestCase): @@ -159,18 +153,18 @@ class OpenEndedModuleTest(unittest.TestCase): max_score = 4 static_data = { - 'max_attempts': 20, - 'prompt': prompt, - 'rubric': rubric, - 'max_score': max_score, - 'display_name': 'Name', - 'accept_file_upload': False, - 'rewrite_content_links' : "", - 'close_date': None, - 's3_interface' : test_util_open_ended.S3_INTERFACE, - 'open_ended_grading_interface' : test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, - 'skip_basic_checks' : False, - } + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + 'display_name': 'Name', + 'accept_file_upload': False, + 'rewrite_content_links': "", + 'close_date': None, + 's3_interface': test_util_open_ended.S3_INTERFACE, + 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, + 'skip_basic_checks': False, + } oeparam = etree.XML(''' @@ -188,25 +182,26 @@ class OpenEndedModuleTest(unittest.TestCase): self.test_system.location = self.location self.mock_xqueue = MagicMock() self.mock_xqueue.send_to_queue.return_value = (None, "Message") - self.test_system.xqueue = {'interface': self.mock_xqueue, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 1} + self.test_system.xqueue = {'interface': self.mock_xqueue, 'callback_url': '/', 'default_queuename': 'testqueue', + 'waittime': 1} self.openendedmodule = OpenEndedModule(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, self.metadata) def test_message_post(self): get = {'feedback': 'feedback text', - 'submission_id': '1', - 'grader_id': '1', - 'score': 3} + 'submission_id': '1', + 'grader_id': '1', + 'score': 3} qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, - 'submission_time': qtime} + 'submission_time': qtime} contents = { - 'feedback': get['feedback'], - 'submission_id': int(get['submission_id']), - 'grader_id': int(get['grader_id']), - 'score': get['score'], - 'student_info': json.dumps(student_info) - } + 'feedback': get['feedback'], + 'submission_id': int(get['submission_id']), + 'grader_id': int(get['grader_id']), + 'score': get['score'], + 'student_info': json.dumps(student_info) + } result = self.openendedmodule.message_post(get, self.test_system) self.assertTrue(result['success']) @@ -220,13 +215,13 @@ class OpenEndedModuleTest(unittest.TestCase): submission = "This is a student submission" qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, - 'submission_time': qtime} + 'submission_time': qtime} contents = self.openendedmodule.payload.copy() contents.update({ 'student_info': json.dumps(student_info), 'student_response': submission, 'max_score': self.max_score - }) + }) result = self.openendedmodule.send_to_grader(submission, self.test_system) self.assertTrue(result) self.mock_xqueue.send_to_queue.assert_called_with(body=json.dumps(contents), header=ANY) @@ -234,36 +229,36 @@ class OpenEndedModuleTest(unittest.TestCase): def update_score_single(self): self.openendedmodule.new_history_entry("New Entry") score_msg = { - 'correct': True, - 'score': 4, - 'msg': 'Grader Message', - 'feedback': "Grader Feedback" - } + 'correct': True, + 'score': 4, + 'msg': 'Grader Message', + 'feedback': "Grader Feedback" + } get = {'queuekey': "abcd", - 'xqueue_body': score_msg} + 'xqueue_body': score_msg} self.openendedmodule.update_score(get, self.test_system) def update_score_single(self): self.openendedmodule.new_history_entry("New Entry") feedback = { - "success": True, - "feedback": "Grader Feedback" - } + "success": True, + "feedback": "Grader Feedback" + } score_msg = { - 'correct': True, - 'score': 4, - 'msg': 'Grader Message', - 'feedback': json.dumps(feedback), - 'grader_type': 'IN', - 'grader_id': '1', - 'submission_id': '1', - 'success': True, - 'rubric_scores': [0], - 'rubric_scores_complete': True, - 'rubric_xml': etree.tostring(self.rubric) - } + 'correct': True, + 'score': 4, + 'msg': 'Grader Message', + 'feedback': json.dumps(feedback), + 'grader_type': 'IN', + 'grader_id': '1', + 'submission_id': '1', + 'success': True, + 'rubric_scores': [0], + 'rubric_scores_complete': True, + 'rubric_xml': etree.tostring(self.rubric) + } get = {'queuekey': "abcd", - 'xqueue_body': json.dumps(score_msg)} + 'xqueue_body': json.dumps(score_msg)} self.openendedmodule.update_score(get, self.test_system) def test_latest_post_assessment(self): @@ -296,18 +291,18 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): metadata = {'attempts': '10', 'max_score': max_score} static_data = { - 'max_attempts': 20, - 'prompt': prompt, - 'rubric': rubric, - 'max_score': max_score, - 'display_name': 'Name', - 'accept_file_upload' : False, - 'rewrite_content_links' : "", - 'close_date' : "", - 's3_interface' : test_util_open_ended.S3_INTERFACE, - 'open_ended_grading_interface' : test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, - 'skip_basic_checks' : False, - } + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + 'display_name': 'Name', + 'accept_file_upload': False, + 'rewrite_content_links': "", + 'close_date': "", + 's3_interface': test_util_open_ended.S3_INTERFACE, + 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, + 'skip_basic_checks': False, + } oeparam = etree.XML(''' @@ -329,23 +324,23 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): ''' task_xml2 = ''' - - Enter essay here. - This is the answer. - {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} - - ''' + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''' definition = {'prompt': etree.XML(prompt), 'rubric': etree.XML(rubric), 'task_xml': [task_xml1, task_xml2]} descriptor = Mock() def setUp(self): self.test_system = test_system() - self.combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - self.definition, - self.descriptor, - static_data = self.static_data, - metadata=self.metadata) + self.combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 362b73df67..a7f2a9fdfe 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -10,8 +10,8 @@ from . import test_system import test_util_open_ended -class SelfAssessmentTest(unittest.TestCase): +class SelfAssessmentTest(unittest.TestCase): rubric = ''' Response Quality @@ -24,7 +24,7 @@ class SelfAssessmentTest(unittest.TestCase): 'prompt': prompt, 'submitmessage': 'Shall we submit now?', 'hintprompt': 'Consider this...', - } + } location = Location(["i4x", "edX", "sa_test", "selfassessment", "SampleQuestion"]) @@ -41,22 +41,22 @@ class SelfAssessmentTest(unittest.TestCase): 'attempts': 2}) static_data = { - 'max_attempts': 10, - 'rubric': etree.XML(self.rubric), - 'prompt': self.prompt, - 'max_score': 1, - 'display_name': "Name", - 'accept_file_upload': False, - 'close_date': None, - 's3_interface' : test_util_open_ended.S3_INTERFACE, - 'open_ended_grading_interface' : test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, - 'skip_basic_checks' : False, - } + 'max_attempts': 10, + 'rubric': etree.XML(self.rubric), + 'prompt': self.prompt, + 'max_score': 1, + 'display_name': "Name", + 'accept_file_upload': False, + 'close_date': None, + 's3_interface': test_util_open_ended.S3_INTERFACE, + 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, + 'skip_basic_checks': False, + } self.module = SelfAssessmentModule(test_system(), self.location, - self.definition, self.descriptor, - static_data, - state, metadata=self.metadata) + self.definition, self.descriptor, + static_data, + state, metadata=self.metadata) def test_get_html(self): html = self.module.get_html(self.module.system) @@ -64,14 +64,15 @@ class SelfAssessmentTest(unittest.TestCase): def test_self_assessment_flow(self): responses = {'assessment': '0', 'score_list[]': ['0', '0']} + def get_fake_item(name): return responses[name] - def get_data_for_location(self,location,student): + def get_data_for_location(self, location, student): return { - 'count_graded' : 0, - 'count_required' : 0, - 'student_sub_count': 0, + 'count_graded': 0, + 'count_required': 0, + 'student_sub_count': 0, } mock_query_dict = MagicMock() @@ -82,20 +83,19 @@ class SelfAssessmentTest(unittest.TestCase): self.assertEqual(self.module.get_score()['score'], 0) - self.module.save_answer({'student_answer': "I am an answer"}, + self.module.save_answer({'student_answer': "I am an answer"}, self.module.system) self.assertEqual(self.module.state, self.module.ASSESSING) self.module.save_assessment(mock_query_dict, self.module.system) self.assertEqual(self.module.state, self.module.DONE) - d = self.module.reset({}) self.assertTrue(d['success']) self.assertEqual(self.module.state, self.module.INITIAL) # if we now assess as right, skip the REQUEST_HINT state - self.module.save_answer({'student_answer': 'answer 4'}, + self.module.save_answer({'student_answer': 'answer 4'}, self.module.system) responses['assessment'] = '1' self.module.save_assessment(mock_query_dict, self.module.system) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 8d1fcd30ce..db580f1e0e 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,14 +1,14 @@ OPEN_ENDED_GRADING_INTERFACE = { - 'url' : 'http://127.0.0.1:3033/', - 'username' : 'incorrect', - 'password' : 'incorrect', - 'staff_grading' : 'staff_grading', - 'peer_grading' : 'peer_grading', - 'grading_controller' : 'grading_controller' + 'url': 'http://127.0.0.1:3033/', + 'username': 'incorrect', + 'password': 'incorrect', + 'staff_grading': 'staff_grading', + 'peer_grading': 'peer_grading', + 'grading_controller': 'grading_controller' } S3_INTERFACE = { - 'aws_access_key' : "", - 'aws_secret_key' : "", - "aws_bucket_name" : "", + 'aws_access_key': "", + 'aws_secret_key': "", + "aws_bucket_name": "", } \ No newline at end of file diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index b4ca20079f..d67e2816d5 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -22,7 +22,7 @@ NOTIFICATION_TYPES = ( ('staff_needs_to_grade', 'staff_grading', 'Staff Grading'), ('new_student_grading_to_view', 'open_ended_problems', 'Problems you have submitted'), ('flagged_submissions_exist', 'open_ended_flagged_problems', 'Flagged Submissions') - ) +) def staff_grading_notifications(course, user): @@ -46,7 +46,9 @@ def staff_grading_notifications(course, user): #Non catastrophic error, so no real action notifications = {} #This is a dev_facing_error - log.info("Problem with getting notifications from staff grading service for course {0} user {1}.".format(course_id, student_id)) + log.info( + "Problem with getting notifications from staff grading service for course {0} user {1}.".format(course_id, + student_id)) if pending_grading: img_path = "/static/images/grading_notification.png" @@ -80,7 +82,9 @@ def peer_grading_notifications(course, user): #Non catastrophic error, so no real action notifications = {} #This is a dev_facing_error - log.info("Problem with getting notifications from peer grading service for course {0} user {1}.".format(course_id, student_id)) + log.info( + "Problem with getting notifications from peer grading service for course {0} user {1}.".format(course_id, + student_id)) if pending_grading: img_path = "/static/images/grading_notification.png" @@ -105,7 +109,9 @@ def combined_notifications(course, user): return notification_dict min_time_to_query = user.last_login - last_module_seen = StudentModule.objects.filter(student=user, course_id=course_id, modified__gt=min_time_to_query).values('modified').order_by('-modified') + last_module_seen = StudentModule.objects.filter(student=user, course_id=course_id, + modified__gt=min_time_to_query).values('modified').order_by( + '-modified') last_module_seen_count = last_module_seen.count() if last_module_seen_count > 0: @@ -117,7 +123,8 @@ def combined_notifications(course, user): img_path = "" try: - controller_response = controller_qs.check_combined_notifications(course.id, student_id, user_is_staff, last_time_viewed) + controller_response = controller_qs.check_combined_notifications(course.id, student_id, user_is_staff, + last_time_viewed) log.debug(controller_response) notifications = json.loads(controller_response) if notifications['success']: @@ -127,7 +134,9 @@ def combined_notifications(course, user): #Non catastrophic error, so no real action notifications = {} #This is a dev_facing_error - log.exception("Problem with getting notifications from controller query service for course {0} user {1}.".format(course_id, student_id)) + log.exception( + "Problem with getting notifications from controller query service for course {0} user {1}.".format( + course_id, student_id)) if pending_grading: img_path = "/static/images/grading_notification.png" @@ -151,7 +160,8 @@ def set_value_in_cache(student_id, course_id, notification_type, value): def create_key_name(student_id, course_id, notification_type): - key_name = "{prefix}{type}_{course}_{student}".format(prefix=KEY_PREFIX, type=notification_type, course=course_id, student=student_id) + key_name = "{prefix}{type}_{course}_{student}".format(prefix=KEY_PREFIX, type=notification_type, course=course_id, + student=student_id) return key_name diff --git a/lms/djangoapps/open_ended_grading/staff_grading.py b/lms/djangoapps/open_ended_grading/staff_grading.py index e39b26da56..fad5268294 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading.py +++ b/lms/djangoapps/open_ended_grading/staff_grading.py @@ -15,6 +15,7 @@ class StaffGrading(object): """ Wrap up functionality for staff grading of submissions--interface exposes get_html, ajax views. """ + def __init__(self, course): self.course = course diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 79b92dffba..91138bf685 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -20,10 +20,12 @@ log = logging.getLogger(__name__) STAFF_ERROR_MESSAGE = 'Could not contact the external grading server. Please contact the development team. If you do not have a point of contact, you can contact Vik at vik@edx.org.' + class MockStaffGradingService(object): """ A simple mockup of a staff grading service, testing. """ + def __init__(self): self.cnt = 0 @@ -43,15 +45,18 @@ class MockStaffGradingService(object): def get_problem_list(self, course_id, grader_id): self.cnt += 1 return json.dumps({'success': True, - 'problem_list': [ - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', - 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5, 'min_for_ml': 10}), - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', - 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5, 'min_for_ml': 10}) - ]}) + 'problem_list': [ + json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', + 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5, + 'min_for_ml': 10}), + json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', + 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5, + 'min_for_ml': 10}) + ]}) - def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, submission_flagged): + def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, + submission_flagged): return self.get_next(course_id, 'fake location', grader_id) @@ -59,6 +64,7 @@ class StaffGradingService(GradingService): """ Interface to staff grading backend. """ + def __init__(self, config): config['system'] = ModuleSystem(None, None, None, render_to_string, None) super(StaffGradingService, self).__init__(config) @@ -109,12 +115,13 @@ class StaffGradingService(GradingService): GradingServiceError: something went wrong with the connection. """ response = self.get(self.get_next_url, - params={'location': location, - 'grader_id': grader_id}) + params={'location': location, + 'grader_id': grader_id}) return json.dumps(self._render_rubric(response)) - def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, submission_flagged): + def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, + submission_flagged): """ Save a score and feedback for a submission. @@ -253,14 +260,14 @@ def get_problem_list(request, course_id): try: response = staff_grading_service().get_problem_list(course_id, unique_id_for_user(request.user)) return HttpResponse(response, - mimetype="application/json") + mimetype="application/json") except GradingServiceError: #This is a dev_facing_error log.exception("Error from staff grading service in open ended grading. server url: {0}" - .format(staff_grading_service().url)) + .format(staff_grading_service().url)) #This is a staff_facing_error return HttpResponse(json.dumps({'success': False, - 'error': STAFF_ERROR_MESSAGE})) + 'error': STAFF_ERROR_MESSAGE})) def _get_next(course_id, grader_id, location): @@ -272,7 +279,7 @@ def _get_next(course_id, grader_id, location): except GradingServiceError: #This is a dev facing error log.exception("Error from staff grading service in open ended grading. server url: {0}" - .format(staff_grading_service().url)) + .format(staff_grading_service().url)) #This is a staff_facing_error return json.dumps({'success': False, 'error': STAFF_ERROR_MESSAGE}) @@ -297,7 +304,7 @@ def save_grade(request, course_id): if request.method != 'POST': raise Http404 - required = set(['score', 'feedback', 'submission_id', 'location','submission_flagged', 'rubric_scores[]']) + required = set(['score', 'feedback', 'submission_id', 'location', 'submission_flagged', 'rubric_scores[]']) actual = set(request.POST.keys()) missing = required - actual if len(missing) > 0: @@ -307,22 +314,23 @@ def save_grade(request, course_id): grader_id = unique_id_for_user(request.user) p = request.POST - location = p['location'] - skipped = 'skipped' in p + skipped = 'skipped' in p try: result_json = staff_grading_service().save_grade(course_id, - grader_id, - p['submission_id'], - p['score'], - p['feedback'], - skipped, - p.getlist('rubric_scores[]'), - p['submission_flagged']) + grader_id, + p['submission_id'], + p['score'], + p['feedback'], + skipped, + p.getlist('rubric_scores[]'), + p['submission_flagged']) except GradingServiceError: #This is a dev_facing_error - log.exception("Error saving grade in the staff grading interface in open ended grading. Request: {0} Course ID: {1}".format(request, course_id)) + log.exception( + "Error saving grade in the staff grading interface in open ended grading. Request: {0} Course ID: {1}".format( + request, course_id)) #This is a staff_facing_error return _err_response(STAFF_ERROR_MESSAGE) @@ -330,13 +338,16 @@ def save_grade(request, course_id): result = json.loads(result_json) except ValueError: #This is a dev_facing_error - log.exception("save_grade returned broken json in the staff grading interface in open ended grading: {0}".format(result_json)) + log.exception( + "save_grade returned broken json in the staff grading interface in open ended grading: {0}".format( + result_json)) #This is a staff_facing_error return _err_response(STAFF_ERROR_MESSAGE) if not result.get('success', False): #This is a dev_facing_error - log.warning('Got success=False from staff grading service in open ended grading. Response: {0}'.format(result_json)) + log.warning( + 'Got success=False from staff grading service in open ended grading. Response: {0}'.format(result_json)) return _err_response(STAFF_ERROR_MESSAGE) # Ok, save_grade seemed to work. Get the next submission to grade. diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index d452883ebb..64123605ce 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -7,7 +7,7 @@ django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/open from django.test import TestCase from open_ended_grading import staff_grading_service from xmodule.open_ended_grading_classes import peer_grading_service -from xmodule import peer_grading_module +from xmodule import peer_grading_module from django.core.urlresolvers import reverse from django.contrib.auth.models import Group @@ -22,6 +22,7 @@ from xmodule.x_module import ModuleSystem from mitxmako.shortcuts import render_to_string import logging + log = logging.getLogger(__name__) from django.test.utils import override_settings from django.http import QueryDict @@ -36,6 +37,7 @@ class TestStaffGradingService(ct.PageLoader): access control and error handling logic -- all the actual work is on the backend. ''' + def setUp(self): xmodule.modulestore.django._MODULESTORES = {} @@ -50,6 +52,7 @@ class TestStaffGradingService(ct.PageLoader): self.course_id = "edX/toy/2012_Fall" self.toy = modulestore().get_course(self.course_id) + def make_instructor(course): group_name = _course_staff_group_name(course.location) g = Group.objects.create(name=group_name) @@ -130,6 +133,7 @@ class TestPeerGradingService(ct.PageLoader): access control and error handling logic -- all the actual work is on the backend. ''' + def setUp(self): xmodule.modulestore.django._MODULESTORES = {} @@ -148,11 +152,12 @@ class TestPeerGradingService(ct.PageLoader): self.mock_service = peer_grading_service.MockPeerGradingService() self.system = ModuleSystem(location, None, None, render_to_string, None, - s3_interface = test_util_open_ended.S3_INTERFACE, - open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE + s3_interface=test_util_open_ended.S3_INTERFACE, + open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE ) self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system) - self.peer_module = peer_grading_module.PeerGradingModule(self.system, location, "", self.descriptor) + self.peer_module = peer_grading_module.PeerGradingModule(self.system, location, "", + self.descriptor) self.peer_module.peer_gs = self.mock_service self.logout() @@ -175,18 +180,20 @@ class TestPeerGradingService(ct.PageLoader): def test_save_grade_success(self): data = { - 'rubric_scores[]': [0, 0], - 'location': self.location, - 'submission_id': 1, - 'submission_key': 'fake key', - 'score': 2, - 'feedback': 'feedback', - 'submission_flagged': 'false' - } + 'rubric_scores[]': [0, 0], + 'location': self.location, + 'submission_id': 1, + 'submission_key': 'fake key', + 'score': 2, + 'feedback': 'feedback', + 'submission_flagged': 'false' + } qdict = MagicMock() + def fake_get_item(key): return data[key] + qdict.__getitem__.side_effect = fake_get_item qdict.getlist = fake_get_item qdict.keys = data.keys @@ -237,18 +244,20 @@ class TestPeerGradingService(ct.PageLoader): def test_save_calibration_essay_success(self): data = { - 'rubric_scores[]': [0, 0], - 'location': self.location, - 'submission_id': 1, - 'submission_key': 'fake key', - 'score': 2, - 'feedback': 'feedback', - 'submission_flagged': 'false' - } + 'rubric_scores[]': [0, 0], + 'location': self.location, + 'submission_id': 1, + 'submission_key': 'fake key', + 'score': 2, + 'feedback': 'feedback', + 'submission_flagged': 'false' + } qdict = MagicMock() + def fake_get_item(key): return data[key] + qdict.__getitem__.side_effect = fake_get_item qdict.getlist = fake_get_item qdict.keys = data.keys diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 55e8088c3f..2e7f429429 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -50,22 +50,24 @@ def _reverse_without_slash(url_name, course_id): ajax_url = reverse(url_name, kwargs={'course_id': course_id}) return ajax_url + DESCRIPTION_DICT = { - 'Peer Grading': "View all problems that require peer assessment in this particular course.", - 'Staff Grading': "View ungraded submissions submitted by students for the open ended problems in the course.", - 'Problems you have submitted': "View open ended problems that you have previously submitted for grading.", - 'Flagged Submissions': "View submissions that have been flagged by students as inappropriate." - } + 'Peer Grading': "View all problems that require peer assessment in this particular course.", + 'Staff Grading': "View ungraded submissions submitted by students for the open ended problems in the course.", + 'Problems you have submitted': "View open ended problems that you have previously submitted for grading.", + 'Flagged Submissions': "View submissions that have been flagged by students as inappropriate." +} ALERT_DICT = { - 'Peer Grading': "New submissions to grade", - 'Staff Grading': "New submissions to grade", - 'Problems you have submitted': "New grades have been returned", - 'Flagged Submissions': "Submissions have been flagged for review" - } + 'Peer Grading': "New submissions to grade", + 'Staff Grading': "New submissions to grade", + 'Problems you have submitted': "New grades have been returned", + 'Flagged Submissions': "Submissions have been flagged for review" +} STUDENT_ERROR_MESSAGE = "Error occured while contacting the grading service. Please notify course staff." STAFF_ERROR_MESSAGE = "Error occured while contacting the grading service. Please notify the development team. If you do not have a point of contact, please email Vik at vik@edx.org" + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def staff_grading(request, course_id): """ @@ -92,10 +94,10 @@ def peer_grading(request, course_id): #Get the current course course = get_course_with_access(request.user, course_id, 'load') course_id_parts = course.id.split("/") - false_dict = [False,"False", "false", "FALSE"] + false_dict = [False, "False", "false", "FALSE"] #Reverse the base course url - base_course_url = reverse('courses') + base_course_url = reverse('courses') try: #TODO: This will not work with multiple runs of a course. Make it work. The last key in the Location passed #to get_items is called revision. Is this the same as run? @@ -147,7 +149,7 @@ def student_problem_list(request, course_id): success = False error_text = "" problem_list = [] - base_course_url = reverse('courses') + base_course_url = reverse('courses') try: problem_list_json = controller_qs.get_grading_status_list(course_id, unique_id_for_user(request.user)) @@ -174,7 +176,7 @@ def student_problem_list(request, course_id): except: #This is a student_facing_error eta_string = "Error getting ETA." - problem_list[i].update({'eta_string' : eta_string}) + problem_list[i].update({'eta_string': eta_string}) except GradingServiceError: #This is a student_facing_error @@ -215,7 +217,7 @@ def flagged_problem_list(request, course_id): success = False error_text = "" problem_list = [] - base_course_url = reverse('courses') + base_course_url = reverse('courses') try: problem_list_json = controller_qs.get_flagged_problem_list(course_id) @@ -243,14 +245,14 @@ def flagged_problem_list(request, course_id): ajax_url = _reverse_with_slash('open_ended_flagged_problems', course_id) context = { - 'course': course, - 'course_id': course_id, - 'ajax_url': ajax_url, - 'success': success, - 'problem_list': problem_list, - 'error_text': error_text, - # Checked above - 'staff_access': True, + 'course': course, + 'course_id': course_id, + 'ajax_url': ajax_url, + 'success': success, + 'problem_list': problem_list, + 'error_text': error_text, + # Checked above + 'staff_access': True, } return render_to_response('open_ended_problems/open_ended_flagged_problems.html', context) @@ -305,7 +307,7 @@ def combined_notifications(request, course_id): } return render_to_response('open_ended_problems/combined_notifications.html', - combined_dict + combined_dict ) @@ -318,13 +320,14 @@ def take_action_on_flags(request, course_id): if request.method != 'POST': raise Http404 - required = ['submission_id', 'action_type', 'student_id'] for key in required: if key not in request.POST: #This is a staff_facing_error - return HttpResponse(json.dumps({'success': False, 'error': STAFF_ERROR_MESSAGE + 'Missing key {0} from submission. Please reload and try again.'.format(key)}), - mimetype="application/json") + return HttpResponse(json.dumps({'success': False, + 'error': STAFF_ERROR_MESSAGE + 'Missing key {0} from submission. Please reload and try again.'.format( + key)}), + mimetype="application/json") p = request.POST submission_id = p['submission_id'] @@ -338,5 +341,7 @@ def take_action_on_flags(request, course_id): return HttpResponse(response, mimetype="application/json") except GradingServiceError: #This is a dev_facing_error - log.exception("Error taking action on flagged peer grading submissions, submission_id: {0}, action_type: {1}, grader_id: {2}".format(submission_id, action_type, grader_id)) + log.exception( + "Error taking action on flagged peer grading submissions, submission_id: {0}, action_type: {1}, grader_id: {2}".format( + submission_id, action_type, grader_id)) return _err_response(STAFF_ERROR_MESSAGE) From fe0447093c0b32fb2ab00f0ac52079d8c733983a Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 14:00:18 -0400 Subject: [PATCH 09/17] add management command to regrade partial-credit problems affected by get_npoints bug. --- .../management/commands/regrade_partial.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 lms/djangoapps/courseware/management/commands/regrade_partial.py diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py new file mode 100644 index 0000000000..7bfcb16913 --- /dev/null +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -0,0 +1,90 @@ +import json +import logging +from optparse import make_option + +from django.core.management.base import BaseCommand # , CommandError + +from courseware.models import StudentModule +from capa.correctmap import CorrectMap + +# +# This is aimed at fixing a temporary problem encountered where partial credit was awarded for +# code problems, but the resulting score (or grade) was mistakenly set to zero +# because of a bug in CorrectMap.get_npoints(). +# +# The fix here is to recalculate the score/grade based on the partial credit. +# To narrow down the set of problems that might need fixing, the StudentModule +# objects to be checked is filtered down to those: +# +# grade=0.0 (the grade must have been zeroed out) +# created < '2013-03-08 05:19:00' (the problem must have been answered before the fix was installed) +# modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) +# state like '%"npoints": 0.%' (the problem must have some form of partial credit). +# + +log = logging.getLogger(__name__) + +class Command(BaseCommand): + + num_visited = 0 + num_changed = 0 + + option_list = BaseCommand.option_list + ( + make_option('--save', + action='store_true', + dest='save_changes', + default=False, + help='Persist the changes that were encountered. If not set, no changes are saved.'), ) + + def fix_studentmodules(self, save_changes): + modules = StudentModule.objects.filter(# module_type='problem', + modified__gt='2013-03-07 20:18:00', + created__lt='2013-03-08 05:19:00', + state__contains='"npoints": 0.', + grade=0.0) + for module in modules: + self.fix_studentmodule(module, save_changes) + + def fix_studentmodule(self, module, save_changes): + module_state = module.state + if module_state is None: + log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + return + + state_dict = json.loads(module_state) + self.num_visited += 1 + + correct_map = CorrectMap() + if 'correct_map' in state_dict: + correct_map.set_dict(state_dict['correct_map']) + + correct = 0 + for key in correct_map: + correct += correct_map.get_npoints(key) + + if module.grade == correct: + log.info("Grade matches for {type} module {id} for student {student} in course {course_id}".format( + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + elif save_changes: + log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( + module.grade, correct, + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + module.grade = correct + module.save() + self.num_changed += 1 + else: + log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( + module.grade, correct, + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + self.num_changed += 1 + + def handle(self, **options): + save_changes = 'save_changes' in options and options['save_changes'] + + log.info("Starting run: save_changes = {0}".format(save_changes)) + + self.fix_studentmodules(save_changes) + + log.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) + From c40ab310c3310a0be8e718afa3e913dcc0212082 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 12 Mar 2013 14:24:42 -0400 Subject: [PATCH 10/17] Blank line fixes --- .../combined_open_ended_rubric.py | 8 -------- .../open_ended_grading_classes/open_ended_module.py | 3 --- .../xmodule/open_ended_grading_classes/openendedchild.py | 3 --- .../open_ended_grading_classes/self_assessment_module.py | 2 -- 4 files changed, 16 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py index f4ea7648a1..287aeb5c24 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py @@ -133,7 +133,6 @@ class CombinedOpenEndedRubric(object): categories.append(self.extract_category(category)) return categories - def extract_category(self, category): ''' construct an individual category @@ -235,7 +234,6 @@ class CombinedOpenEndedRubric(object): }) return html - @staticmethod def validate_options(options): ''' @@ -328,9 +326,3 @@ class CombinedOpenEndedRubric(object): tuples.append([0, category, score, [], []]) tup_ind = len(tuples) - 1 return tuples, tup_ind - - - - - - diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index b16e7f5313..fc53a62c06 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -77,7 +77,6 @@ class OpenEndedModule(openendedchild.OpenEndedChild): self.send_to_grader(self.latest_answer(), system) self.created = False - def _parse(self, oeparam, prompt, rubric, system): ''' Parse OpenEndedResponse XML: @@ -270,7 +269,6 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return True - def get_answers(self): """ Gets and shows the answer for this problem. @@ -415,7 +413,6 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return feedback_template, rubric_scores - def _parse_score_msg(self, score_msg, system, join_feedback=True): """ Grader reply is a JSON-dump of the following dict diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 91dec5943d..922a4f9b77 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -490,6 +490,3 @@ class OpenEndedChild(object): eta_string = "" return eta_string - - - diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index a25d81eeae..8911e2890f 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -73,7 +73,6 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): html = system.render_template('{0}/self_assessment_prompt.html'.format(self.TEMPLATE_DIR), context) return html - def handle_ajax(self, dispatch, get, system): """ This is called by courseware.module_render, to handle an AJAX call. @@ -159,7 +158,6 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): return system.render_template('{0}/self_assessment_hint.html'.format(self.TEMPLATE_DIR), context) - def save_answer(self, get, system): """ After the answer is submitted, show the rubric. From 70af75e8525f6d0e9cdfe1ec39396c0852528d86 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 12 Mar 2013 14:34:39 -0400 Subject: [PATCH 11/17] Don't run pylint on migrations, and it's ok to have a global name 'log' --- .pylintrc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pylintrc b/.pylintrc index ce2f2e3b87..4357885133 100644 --- a/.pylintrc +++ b/.pylintrc @@ -12,7 +12,7 @@ profile=no # Add files or directories to the blacklist. They should be base names, not # paths. -ignore=CVS +ignore=CVS, migrations # Pickle collected data for later comparisons. persistent=yes @@ -43,7 +43,7 @@ disable=E1102,W0142 output-format=text # Include message's id in output -include-ids=no +include-ids=yes # Put messages in a separate file for each module / package specified on the # command line instead of printing them on stdout. Reports (if any) will be @@ -97,7 +97,7 @@ bad-functions=map,filter,apply,input module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ # Regular expression which should only match correct module level names -const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__)|log)$ # Regular expression which should only match correct class names class-rgx=[A-Z_][a-zA-Z0-9]+$ From 6aed059d5ff70e18335cf7e12e3a5812108060a4 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 14:55:27 -0400 Subject: [PATCH 12/17] Change date and remove filtering by grade. Add check for student_answers. --- .../management/commands/regrade_partial.py | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py index 7bfcb16913..b997cff674 100644 --- a/lms/djangoapps/courseware/management/commands/regrade_partial.py +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -16,8 +16,7 @@ from capa.correctmap import CorrectMap # To narrow down the set of problems that might need fixing, the StudentModule # objects to be checked is filtered down to those: # -# grade=0.0 (the grade must have been zeroed out) -# created < '2013-03-08 05:19:00' (the problem must have been answered before the fix was installed) +# created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, on Prod and Edge) # modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) # state like '%"npoints": 0.%' (the problem must have some form of partial credit). # @@ -39,15 +38,16 @@ class Command(BaseCommand): def fix_studentmodules(self, save_changes): modules = StudentModule.objects.filter(# module_type='problem', modified__gt='2013-03-07 20:18:00', - created__lt='2013-03-08 05:19:00', - state__contains='"npoints": 0.', - grade=0.0) + created__lt='2013-03-08 15:45:00', + state__contains='"npoints": 0.') + for module in modules: - self.fix_studentmodule(module, save_changes) + self.fix_studentmodule_grade(module, save_changes) - def fix_studentmodule(self, module, save_changes): + def fix_studentmodule_grade(self, module, save_changes): module_state = module.state if module_state is None: + # not likely, since we filter on it. But in general... log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) return @@ -55,32 +55,50 @@ class Command(BaseCommand): state_dict = json.loads(module_state) self.num_visited += 1 + # LoncapaProblem.get_score() checks student_answers -- if there are none, we will return a grade of 0 + # Check that this is the case, but do so sooner, before we do any of the other grading work. + student_answers = state_dict['student_answers'] + if (not student_answers) or len(student_answers) == 0: + # we should not have a grade here: + if module.grade != 0: + log.error("No answer found but grade {grade} exists for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + else: + log.debug("No answer and no grade found for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + return + + # load into a CorrectMap, as done in LoncapaProblem.__init__(): correct_map = CorrectMap() if 'correct_map' in state_dict: correct_map.set_dict(state_dict['correct_map']) + # calculate score the way LoncapaProblem.get_score() works, by deferring to CorrectMap's get_npoints implementation. correct = 0 for key in correct_map: correct += correct_map.get_npoints(key) if module.grade == correct: - log.info("Grade matches for {type} module {id} for student {student} in course {course_id}".format( - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + # nothing to change + log.debug("Grade matches for {type} module {id} for student {student} in course {course_id}".format( + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) elif save_changes: + # make the change log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( module.grade, correct, - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) module.grade = correct module.save() self.num_changed += 1 else: + # don't make the change, but log that the change would be made log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( module.grade, correct, - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) self.num_changed += 1 def handle(self, **options): - save_changes = 'save_changes' in options and options['save_changes'] + save_changes = options['save_changes'] log.info("Starting run: save_changes = {0}".format(save_changes)) From e5ee0a77eddb129efcfd4d9a7aaa28eb11d94fe9 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 12 Mar 2013 15:24:10 -0400 Subject: [PATCH 13/17] hot fix for delete_item. The 'delete_all_versions' is useful in that it is set to true when doing explicit deletes as opposed to 'delete draft'. If delete_all_versions=False, then don't remove the deleted item from the parent children collection --- .../contentstore/tests/test_contentstore.py | 2 +- cms/djangoapps/contentstore/views.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 99ef1169b1..9d533dffed 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -112,7 +112,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertTrue(sequential.location.url() in chapter.definition['children']) self.client.post(reverse('delete_item'), - json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), + json.dumps({'id': sequential.location.url(), 'delete_children':'true', 'delete_all_versions':'true'}), "application/json") found = False diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index c2c80106fa..6566350f8d 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -643,15 +643,15 @@ def delete_item(request): modulestore('direct').delete_item(item.location) # cdodge: we need to remove our parent's pointer to us so that it is no longer dangling + if delete_all_versions: + parent_locs = modulestore('direct').get_parent_locations(item_loc, None) - parent_locs = modulestore('direct').get_parent_locations(item_loc, None) - - for parent_loc in parent_locs: - parent = modulestore('direct').get_item(parent_loc) - item_url = item_loc.url() - if item_url in parent.definition["children"]: - parent.definition["children"].remove(item_url) - modulestore('direct').update_children(parent.location, parent.definition["children"]) + for parent_loc in parent_locs: + parent = modulestore('direct').get_item(parent_loc) + item_url = item_loc.url() + if item_url in parent.definition["children"]: + parent.definition["children"].remove(item_url) + modulestore('direct').update_children(parent.location, parent.definition["children"]) return HttpResponse() From 47e708e713735fc94cb38f2096057d36cc7410e3 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 15:49:48 -0400 Subject: [PATCH 14/17] fix pylint and pep8 warnings --- .../management/commands/regrade_partial.py | 104 ++++++++++-------- 1 file changed, 59 insertions(+), 45 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py index b997cff674..c24f84ddfc 100644 --- a/lms/djangoapps/courseware/management/commands/regrade_partial.py +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -1,33 +1,36 @@ +''' +This is a one-off command aimed at fixing a temporary problem encountered where partial credit was awarded for +code problems, but the resulting score (or grade) was mistakenly set to zero because of a bug in +CorrectMap.get_npoints(). +''' + import json import logging from optparse import make_option -from django.core.management.base import BaseCommand # , CommandError +from django.core.management.base import BaseCommand from courseware.models import StudentModule from capa.correctmap import CorrectMap -# -# This is aimed at fixing a temporary problem encountered where partial credit was awarded for -# code problems, but the resulting score (or grade) was mistakenly set to zero -# because of a bug in CorrectMap.get_npoints(). -# -# The fix here is to recalculate the score/grade based on the partial credit. -# To narrow down the set of problems that might need fixing, the StudentModule -# objects to be checked is filtered down to those: -# -# created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, on Prod and Edge) -# modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) -# state like '%"npoints": 0.%' (the problem must have some form of partial credit). -# +LOG = logging.getLogger(__name__) -log = logging.getLogger(__name__) class Command(BaseCommand): - + ''' + The fix here is to recalculate the score/grade based on the partial credit. + To narrow down the set of problems that might need fixing, the StudentModule + objects to be checked is filtered down to those: + + created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, + on Prod and Edge) + modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) + state like '%"npoints": 0.%' (the problem must have some form of partial credit). + ''' + num_visited = 0 num_changed = 0 - + option_list = BaseCommand.option_list + ( make_option('--save', action='store_true', @@ -36,73 +39,84 @@ class Command(BaseCommand): help='Persist the changes that were encountered. If not set, no changes are saved.'), ) def fix_studentmodules(self, save_changes): - modules = StudentModule.objects.filter(# module_type='problem', - modified__gt='2013-03-07 20:18:00', + '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' + modules = StudentModule.objects.filter(modified__gt='2013-03-07 20:18:00', created__lt='2013-03-08 15:45:00', state__contains='"npoints": 0.') for module in modules: self.fix_studentmodule_grade(module, save_changes) - + def fix_studentmodule_grade(self, module, save_changes): + ''' Fix the grade assigned to a StudentModule''' module_state = module.state if module_state is None: # not likely, since we filter on it. But in general... - log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + LOG.info("No state found for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) return - + state_dict = json.loads(module_state) self.num_visited += 1 - + # LoncapaProblem.get_score() checks student_answers -- if there are none, we will return a grade of 0 # Check that this is the case, but do so sooner, before we do any of the other grading work. student_answers = state_dict['student_answers'] if (not student_answers) or len(student_answers) == 0: # we should not have a grade here: if module.grade != 0: - log.error("No answer found but grade {grade} exists for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.error("No answer found but grade {grade} exists for {type} module {id} for student {student} " + "in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) else: - log.debug("No answer and no grade found for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.debug("No answer and no grade found for {type} module {id} for student {student} " + "in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) return # load into a CorrectMap, as done in LoncapaProblem.__init__(): correct_map = CorrectMap() if 'correct_map' in state_dict: correct_map.set_dict(state_dict['correct_map']) - - # calculate score the way LoncapaProblem.get_score() works, by deferring to CorrectMap's get_npoints implementation. + + # calculate score the way LoncapaProblem.get_score() works, by deferring to + # CorrectMap's get_npoints implementation. correct = 0 for key in correct_map: correct += correct_map.get_npoints(key) - + if module.grade == correct: # nothing to change - log.debug("Grade matches for {type} module {id} for student {student} in course {course_id}".format( - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.debug("Grade matches for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) elif save_changes: # make the change - log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( - module.grade, correct, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.info("Grade changing from {0} to {1} for {type} module {id} for student {student} " + "in course {course_id}".format(module.grade, correct, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) module.grade = correct module.save() self.num_changed += 1 else: # don't make the change, but log that the change would be made - log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( - module.grade, correct, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.info("Grade would change from {0} to {1} for {type} module {id} for student {student} " + "in course {course_id}".format(module.grade, correct, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) self.num_changed += 1 - + def handle(self, **options): + '''Handle management command request''' + save_changes = options['save_changes'] - log.info("Starting run: save_changes = {0}".format(save_changes)) - + LOG.info("Starting run: save_changes = {0}".format(save_changes)) + self.fix_studentmodules(save_changes) - - log.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) - + + LOG.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) From 174ba0039d88e8f0b1a161b925c9ff653bb1c291 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 12 Mar 2013 12:41:04 -0400 Subject: [PATCH 15/17] Make sure that test-requirements.txt doesn't install versions of libraries that requirements.txt has pinned, and cache pip downloads --- jenkins/test.sh | 5 ++- requirements.txt | 94 +++++++++++++++++++++---------------------- test-requirements.txt | 3 -- 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/jenkins/test.sh b/jenkins/test.sh index 3a572c9808..f8ffab29fc 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -32,10 +32,11 @@ if [ ! -d /mnt/virtualenvs/"$JOB_NAME" ]; then virtualenv /mnt/virtualenvs/"$JOB_NAME" fi +export PIP_DOWNLOAD_CACHE=/mnt/pip-cache + source /mnt/virtualenvs/"$JOB_NAME"/bin/activate pip install -q -r pre-requirements.txt -pip install -q -r test-requirements.txt -yes w | pip install -q -r requirements.txt +yes w | pip install -q -r test-requirements.txt -r requirements.txt rake clobber rake pep8 diff --git a/requirements.txt b/requirements.txt index 204ec85889..3dc732e013 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,61 +1,61 @@ -django==1.4.3 -pip -numpy==1.6.2 -scipy==0.11.0 -Markdown==2.2.1 -pygments==1.5 -lxml==3.0.1 -boto==2.6.0 -mako==0.7.3 -python-memcached==1.48 -python-openid==2.2.5 -path.py -django_debug_toolbar -fs==0.4.0 -beautifulsoup==3.2.1 +-r repo-requirements.txt beautifulsoup4==4.1.3 -feedparser==5.1.3 -requests==0.14.2 -http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz -newrelic==1.8.0.13 -glob2==0.3 -pymongo==2.4.1 -django_nose==1.1 -nosexcover==1.0.7 -rednose==0.3.3 -GitPython==0.3.2.RC1 -mock==0.8.0 -PyYAML==3.10 -South==0.7.6 -pytz==2012h +beautifulsoup==3.2.1 +boto==2.6.0 django-celery==3.0.11 django-countries==1.5 -django-kombu==0.9.4 +django-debug-toolbar-mongo django-followit==0.0.3 django-jasmine==0.3.2 django-keyedcache==1.4-6 +django-kombu==0.9.4 django-mako==0.1.5pre django-masquerade==0.1.6 +django-mptt==0.5.5 django-openid-auth==0.4 django-robots==0.9.1 +django-sekizai==0.6.1 django-ses==0.4.1 django-storages==1.1.5 django-threaded-multihost==1.4-1 -django-sekizai==0.6.1 -django-mptt==0.5.5 -sorl-thumbnail==11.12 -networkx==1.7 -pygraphviz==1.1 --r repo-requirements.txt -nltk==2.0.4 -django-debug-toolbar-mongo -dogstatsd-python==0.2.1 -MySQL-python==1.2.4c1 -sphinx==1.1.3 -factory_boy -Shapely==1.2.16 -ipython==0.13.1 -xmltodict==0.4.1 -paramiko==1.9.0 -Pillow==1.7.8 +django==1.4.3 +django_debug_toolbar +django_nose==1.1 dogapi==1.2.1 +dogstatsd-python==0.2.1 +factory_boy +feedparser==5.1.3 +fs==0.4.0 +GitPython==0.3.2.RC1 +glob2==0.3 +http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz +ipython==0.13.1 +lxml==3.0.1 +mako==0.7.3 +Markdown==2.2.1 +mock==0.8.0 +MySQL-python==1.2.4c1 +networkx==1.7 +newrelic==1.8.0.13 +nltk==2.0.4 +nosexcover==1.0.7 +numpy==1.6.2 +paramiko==1.9.0 +path.py +Pillow==1.7.8 +pip +pygments==1.5 +pygraphviz==1.1 +pymongo==2.4.1 +python-memcached==1.48 +python-openid==2.2.5 +pytz==2012h +PyYAML==3.10 +rednose==0.3.3 +requests==0.14.2 +scipy==0.11.0 +Shapely==1.2.16 +sorl-thumbnail==11.12 +South==0.7.6 +sphinx==1.1.3 +xmltodict==0.4.1 diff --git a/test-requirements.txt b/test-requirements.txt index 4706e239a5..6c3322acbf 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,10 +1,7 @@ -django-nose coverage -nosexcover pylint pep8 lettuce selenium -factory_boy splinter From b05b627c237dc623c02ce5649f71a3d15ac746fd Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 12 Mar 2013 16:10:37 -0400 Subject: [PATCH 16/17] Reduce some more pylint noise --- .pylintrc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4357885133..d1cdbb4780 100644 --- a/.pylintrc +++ b/.pylintrc @@ -33,7 +33,11 @@ load-plugins= # can either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where # it should appear only once). -disable=E1102,W0142 +disable= +# W0141: Used builtin function 'map' +# W0142: Used * or ** magic +# R0903: Too few public methods (1/2) + W0141,W0142,R0903 [REPORTS] @@ -97,7 +101,7 @@ bad-functions=map,filter,apply,input module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ # Regular expression which should only match correct module level names -const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__)|log)$ +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns)$ # Regular expression which should only match correct class names class-rgx=[A-Z_][a-zA-Z0-9]+$ @@ -106,7 +110,7 @@ class-rgx=[A-Z_][a-zA-Z0-9]+$ function-rgx=[a-z_][a-z0-9_]{2,30}$ # Regular expression which should only match correct method names -method-rgx=[a-z_][a-z0-9_]{2,30}$ +method-rgx=([a-z_][a-z0-9_]{2,60}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*)$ # Regular expression which should only match correct instance attribute names attr-rgx=[a-z_][a-z0-9_]{2,30}$ From 47420fe7c5b29d9aa5b3d0724495f70b73c36a4f Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 12 Mar 2013 12:41:04 -0400 Subject: [PATCH 17/17] Make sure that test-requirements.txt doesn't install versions of libraries that requirements.txt has pinned, and cache pip downloads --- jenkins/test.sh | 5 ++- requirements.txt | 94 +++++++++++++++++++++---------------------- test-requirements.txt | 3 -- 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/jenkins/test.sh b/jenkins/test.sh index 3a572c9808..f8ffab29fc 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -32,10 +32,11 @@ if [ ! -d /mnt/virtualenvs/"$JOB_NAME" ]; then virtualenv /mnt/virtualenvs/"$JOB_NAME" fi +export PIP_DOWNLOAD_CACHE=/mnt/pip-cache + source /mnt/virtualenvs/"$JOB_NAME"/bin/activate pip install -q -r pre-requirements.txt -pip install -q -r test-requirements.txt -yes w | pip install -q -r requirements.txt +yes w | pip install -q -r test-requirements.txt -r requirements.txt rake clobber rake pep8 diff --git a/requirements.txt b/requirements.txt index 204ec85889..3dc732e013 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,61 +1,61 @@ -django==1.4.3 -pip -numpy==1.6.2 -scipy==0.11.0 -Markdown==2.2.1 -pygments==1.5 -lxml==3.0.1 -boto==2.6.0 -mako==0.7.3 -python-memcached==1.48 -python-openid==2.2.5 -path.py -django_debug_toolbar -fs==0.4.0 -beautifulsoup==3.2.1 +-r repo-requirements.txt beautifulsoup4==4.1.3 -feedparser==5.1.3 -requests==0.14.2 -http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz -newrelic==1.8.0.13 -glob2==0.3 -pymongo==2.4.1 -django_nose==1.1 -nosexcover==1.0.7 -rednose==0.3.3 -GitPython==0.3.2.RC1 -mock==0.8.0 -PyYAML==3.10 -South==0.7.6 -pytz==2012h +beautifulsoup==3.2.1 +boto==2.6.0 django-celery==3.0.11 django-countries==1.5 -django-kombu==0.9.4 +django-debug-toolbar-mongo django-followit==0.0.3 django-jasmine==0.3.2 django-keyedcache==1.4-6 +django-kombu==0.9.4 django-mako==0.1.5pre django-masquerade==0.1.6 +django-mptt==0.5.5 django-openid-auth==0.4 django-robots==0.9.1 +django-sekizai==0.6.1 django-ses==0.4.1 django-storages==1.1.5 django-threaded-multihost==1.4-1 -django-sekizai==0.6.1 -django-mptt==0.5.5 -sorl-thumbnail==11.12 -networkx==1.7 -pygraphviz==1.1 --r repo-requirements.txt -nltk==2.0.4 -django-debug-toolbar-mongo -dogstatsd-python==0.2.1 -MySQL-python==1.2.4c1 -sphinx==1.1.3 -factory_boy -Shapely==1.2.16 -ipython==0.13.1 -xmltodict==0.4.1 -paramiko==1.9.0 -Pillow==1.7.8 +django==1.4.3 +django_debug_toolbar +django_nose==1.1 dogapi==1.2.1 +dogstatsd-python==0.2.1 +factory_boy +feedparser==5.1.3 +fs==0.4.0 +GitPython==0.3.2.RC1 +glob2==0.3 +http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz +ipython==0.13.1 +lxml==3.0.1 +mako==0.7.3 +Markdown==2.2.1 +mock==0.8.0 +MySQL-python==1.2.4c1 +networkx==1.7 +newrelic==1.8.0.13 +nltk==2.0.4 +nosexcover==1.0.7 +numpy==1.6.2 +paramiko==1.9.0 +path.py +Pillow==1.7.8 +pip +pygments==1.5 +pygraphviz==1.1 +pymongo==2.4.1 +python-memcached==1.48 +python-openid==2.2.5 +pytz==2012h +PyYAML==3.10 +rednose==0.3.3 +requests==0.14.2 +scipy==0.11.0 +Shapely==1.2.16 +sorl-thumbnail==11.12 +South==0.7.6 +sphinx==1.1.3 +xmltodict==0.4.1 diff --git a/test-requirements.txt b/test-requirements.txt index 4706e239a5..6c3322acbf 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,10 +1,7 @@ -django-nose coverage -nosexcover pylint pep8 lettuce selenium -factory_boy splinter